This repository was archived by the owner on Apr 23, 2025. It is now read-only.
GPT2 Checkpoint Additions #658
Merged
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
…rs were missing in the writing process. Also added copy facility to the FileSystem protocol to copy the auxiliary files to the checkpoint location.
BradLarson
reviewed
Aug 19, 2020
BradLarson
approved these changes
Aug 19, 2020
There was a problem hiding this comment.
Thanks for the fix. The GPT-2 naming changes were incorporated into the more significant rework I was doing in PR #631, but it's good to have these fixed while I'm working on that. Also, copying the metadata into the new checkpoint location is a good idea, thanks for suggesting that.
|
Thanks for the review. Deleted the branch. I'll delete the fork shortly. I believe I have some outstanding work still on it that I need to move to another branch. |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
This PR proposes to add some functionality to checkpoint writing for GPT2. One of the layers appeared to have the wrong mapping ("weight" vs "embedding") and another layer in the model was not being written (MultiheadAttention vs MultiheadAttentionGPT2). Output from the modified checkpoint file differed slightly from the checkpoint stored remotely by a few bytes so it is possible some additional information is being written by this change versus the remotely stored version.
All checkpoint writing tests passed successfully. Another test could be added to exercise the writing of auxiliary files if desired.
It's known that new protocols are in process (PR #631) for checkpoint writing so this change is more of a historical completion PR for the pattern first established by the GPT2 model. Hopefully, it will be useful for anyone looking at previous examples of checkpoint writing done in the past.
The work was performed on the 0.10 branch due to some compilation or configuration issues on master. A fallback to 0.10 allowed the work to proceed but hopefully it can be merged down if the change is acceptable.