-
Notifications
You must be signed in to change notification settings - Fork 27.2k
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
Enable different torch dtype in sub models #34873
base: main
Are you sure you want to change the base?
Conversation
The docs for this PR live here. All of your documentation changes will be reflected on that endpoint. The docs are available until 30 days after the last update. |
src/transformers/modeling_utils.py
Outdated
for sub_config_key in config.sub_configs.keys(): | ||
sub_config = getattr(config, sub_config_key) | ||
sub_config.torch_dtype = torch_dtype | ||
elif isinstance(torch_dtype, dict): | ||
for key, curr_dtype in torch_dtype.items(): | ||
if hasattr(config, key): | ||
value = getattr(config, key) | ||
value.torch_dtype = curr_dtype |
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.
if users passes one torch dtype as before, we just use it in all sub-configs. Otherwise a user can either set directly dtypes in configs before loading the model, or indicate a dict torch_dtype
when loading similarly to attn_implementation_dispatch
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, can you add a test showcasing an example usage of this (for example with Llava!)
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.
This is interesting but I am not sure we have everything ready:
- does it work with the
keep-in-float32
attribute as well? - does it work well with model that have enforced param with dtypes?
(Some vision models have this! ) So maybe a little bit of testing is missing
Makes a lot of sense otherwise!
if hasattr(config, key): | ||
value = getattr(config, key) | ||
value.torch_dtype = curr_dtype |
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.
else we should probably raise an error no?
Yeah, this one needs time and I'll come back after the model releases to make sure it works in all cases. Currently it has weird behavior in nested configs where a general text config has an attribute |
Conditions to make the dtype dispatch correctly: use The current design support setting dtype via Added more tests and verified it works when |
What does this PR do?
Fixes #33997. Enables users to use different torch dtypes for each of sub config. For ex load the vision model in full precision and the text model in half precision