Tied word embeddings #1260
Merged
Tied word embeddings #1260
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
🔗 Helpful Links🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/torchchat/1260
Note: Links to docs will display an error until the docs builds have been completed. ✅ No FailuresAs of commit c840070 with merge base 6a2a2e8 ( This comment was automatically generated by Dr. CI and updates every 15 minutes. |
facebook-github-bot
added
the
CLA Signed
This was referenced Oct 3, 2024
Merged
gabe-l-hart
force-pushed
the
TiedWordEmbeddings-1252
branch
3 times, most recently
from
b1e2fff to
254fc51
Compare
|
Sweet and simple, @Gasoonjia for a sanity check |
Jack-Khuu
approved these changes
Oct 7, 2024
gabe-l-hart
force-pushed
the
TiedWordEmbeddings-1252
branch
2 times, most recently
from
e002700 to
ced5d03
Compare
Branch: GraniteCodeSupport Signed-off-by: Gabe Goodhart <ghart@us.ibm.com>
gabe-l-hart
force-pushed
the
TiedWordEmbeddings-1252
branch
from
ced5d03 to
c840070
Compare
|
Thanks for working through the chain @Jack-Khuu! This is the next one up |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Dependencies
This PR is part of a sequence in support of adding Granite Code. It depends on merging the following PRs:
Issues
Closes #1252
Description
This PR adds support for models which have shared weights between the input word embeddings and the output layer (tied word embeddings).
Changes
tie_word_embeddingsparameter toTransformerArgsload_hookto theTransformerclass where, if configured withtie_word_embeddings, theself.outputmodule will share weights with theself.tok_embeddingsmodule.Testing
In conjunction with my other changes for Granite Code, I've been able to validate that the results produced with this logic do produce the expected token sequence.
NOTE: If there's any preferred way to include unit tests along with the PR, please let me know and I can get them added! I don't see a familiar unit test structure in the project at this point, so I've been relying on local ad-hoc testing.