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.
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
Smoke tests with tiny gpt2, fix CCSReporter #149
Smoke tests with tiny gpt2, fix CCSReporter #149
Changes from 8 commits
01f3a2f
73bd2b0
9a824bc
d46bf2a
c124d40
b233517
c1cf9a0
e43d2c5
89fbee5
d876f88
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
basically the deberta model itself does some fp16 operations that aren't supported. but tiny-gpt doesn't. so that why tiny-gpt works.
i'll try to figure that out in another MR. i think we could convert fp16 models to fp32 if its CPU, but seems hacky?
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 actually don't view this as hacky. In general float16 isn't supported on CPUs. We could check the whether we're running on CPU right when we call
AutoModel.from_pretrained
and tell HF to always load the model as float32 in that case, and use the dtype of the checkpoint otherwise.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.
^ just made this change, so if you want to add a deberta test you can. I think tiny gpt2 is probably sufficient for now though
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.
EigenReporterConfig is broken for tiny-gpt2. didn't investigate, would like to push the fix for CCS first
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.
Thanks for noticing this, I just pushed a fix