-
Notifications
You must be signed in to change notification settings - Fork 0
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Split forward functions in DXSM models #74
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good; minor suggestions here.
netam/models.py
Outdated
representation: A tensor of shape (B, L, E) representing the | ||
embedded parent sequences. | ||
Returns: | ||
A tensor of shape (B, L, 1) representing the log level of selection |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Compare with (B, L, 20) below
netam/models.py
Outdated
mask: A tensor of shape (B, L) representing the mask of valid amino acid sites. | ||
|
||
Returns: | ||
A tensor of shape (B, L, 20) representing the log level of selection |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think rather than 20 it should be out_features
which is a kwd arg. It's set to 1 for classic DSNM.
Speaking of which, in the definition of the hyperparameters we have
"output_dim": self.linear.out_features,
We should decide on one or the other of dim or features. What do you prefer?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Intuitively I'd say dim, but maybe features is a more torch-y way of saying it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Beh. Let's honor your intuition.
Makefile
Outdated
@@ -21,6 +21,9 @@ lint: | |||
# exit-zero treats all errors as warnings. The GitHub editor is 127 chars wide | |||
flake8 . --count --max-complexity=30 --max-line-length=127 --statistics --exclude=_ignore | |||
|
|||
lintless: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
feel free to make this the default lint if you'd like.
9281294
to
2992dee
Compare
Splits forward functions into
represent
andpredict
methods, whererepresent
does everything except applying the final linear layer, in preparation for https://github.com/matsengrp/dnsm-experiments-1/issues/28Will close #72.
Will need to be rebased on #73 when that's done/merged.