-
Notifications
You must be signed in to change notification settings - Fork 290
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
Add nanogpt_train to torchbench, and merge with nanogpt_generate into joint folder #1911
Conversation
7cbee8a
to
95cc251
Compare
960f7e3
to
e04b67e
Compare
0956187
to
8c9c30a
Compare
Can we merge nanogpt_train with nanogpt_generate? Their model.py looks similiar and it is better to have a model with both inference and train tests. |
@xuzhao9 by merging, should we combine them into a new |
Yes, if they have the same origin, we should combine them into a new |
nanogpt_generate was included in some CI skips, which need to be renamed too so that we don't break CI
|
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.
LGTM, thanks for merging nanogpt!
@msaroufim these changes shouldn't mess up the dashboard right? |
@drisspg Since the dashboard only aggregates (logs) this change should be safe. Once we land this PR, we will update the torchbench commit hash in pytorch repo. The dashboard's job (logs) will checkout the new torchbench version, run all models under |
@xmfan has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
@pytorchbot merge |
Currently, we have nanogpt_generate, which is the inference only version of https://github.com/karpathy/nanoGPT/blob/master/sample.py. This PR adds the training component, simplified from https://github.com/karpathy/nanoGPT/blob/master/train.py.
Tests