-
Notifications
You must be signed in to change notification settings - Fork 1k
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
Handle multiple tied parameters #1241
Conversation
The documentation is not available anymore as the PR was closed or merged. |
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! This all looks good to me. Test failure is because test_infer_auto_device_map_on_t0pp(self)
needs to use the @require_huggingface_suite
decorator I believe.
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.
Cool! Should there be 🚨 at the top to mention the breaking change?
current_memory_used = 0 | ||
else: | ||
# Otherwise, we replace the tied module by its children. | ||
elif len(tied_params) > 0: |
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.
Scary :)
Co-authored-by: Lysandre Debut <lysandre.debut@reseau.eseo.fr>
This PR reworks the internal of big model inference to handle multiple tied parameters (instead of just a pair). It contains a breaking change in the util function
find_tied_parameters
(which is used once in Transformers, so we will need an update there). This is because its results as a dictionary doesn't really make sense when we can have groups of three/four tied parameters all together.The logic in
infer_auto_device_map
is getting a lot more complicated, so I added averbose
argument to leave plenty of print statements and help future me or others debug what's happening. Tests and integration tests are added as well.