-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
Share a list of weight attributes instead of a single one in TiedLayerSpec API #2035
Share a list of weight attributes instead of a single one in TiedLayerSpec API #2035
Conversation
Co-authored-by: Niklas Muennighoff <n.muennighoff@gmail.com>
@@ -191,7 +203,7 @@ def forward(self, inputs): | |||
self.forward_funcs = [] | |||
self.fwd_map = {} | |||
self.tied_modules = nn.ModuleDict() | |||
self.tied_weight_attrs = {} | |||
self.tied_weight_attrss = {} |
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.
Is the double s
a typo or intentional?
@thomasw21, thanks for the PR and apologies for the delay in reviewing. Can you please provide a clarification? It seems there are two issues being addressed in this PR: (1) supporting user-specified forward function and (2) list of weight attributes. If this is correct, I think it might be better to split into two separate PRs. What do you think? Thanks! |
Indeed! Sorry I've sort of dropped this as well currently, as we've been focusing on other aspects of BigScience. I'll try split the PRs when I get the chance! My bad! |
@tjruwase Are these changes we still want? If so I can revive them with the current develop branch. |
@thomasw21, are you still interested in this PR? Are you fine with #4216 replacing this? |
Hey! I'm fine with #4216 overriding this PR :D |
No description provided.