-
Notifications
You must be signed in to change notification settings - Fork 712
Fix script export_hf_model.py #6246
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
Conversation
🔗 Helpful Links🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/executorch/6246
Note: Links to docs will display an error until the docs builds have been completed. ✅ No FailuresAs of commit e03a61c with merge base 708c6b6 ( This comment was automatically generated by Dr. CI and updates every 15 minutes. |
| "get_bos_id": model.config.bos_token_id, | ||
| "get_eos_id": model.config.eos_token_id, |
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.
@larryliu0820 Is it safe to leave it unset and let the runtime to use the default value? Or it's preferred to set it to some special value e.g. -1, if any of the field is unset by a model?
|
@guangy10 has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
3ffadda to
1d68171
Compare
|
@guangy10 has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
1d68171 to
e03a61c
Compare
|
@guangy10 has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
Having
bos_token_id = None(and other fields) will cause an emitter error for models that doesn't define that field, for example, olmo-1b.This PR avoids emitting with unsupported primitive type by removing the
Nonefields out from the model metadata. The ExecuTorch runtime will assume the default value for those unspecified fields.