Skip to content
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

[DO NOT MERGE] add all level buffer support when computing infer_auto_device_map #2663

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

SunMarc
Copy link
Member

@SunMarc SunMarc commented Apr 12, 2024

What does this PR do ?

This PR adds all level support when computing infer_auto_device_map. This is especially important for persistant buffer that are in the state_dict. If we don't attribute a device to those buffers, we will get error in transformers when we try to load the state_dict. We might hit edge cases with cpu and disk offload.
Usually buffers are defined in a nn.Module that don't have other children modules. However, in a model such as RecurrentGemmaModel, normalize buffer is defined at the level of the model. So when we have model = RecurrentGemmaForCausalLM, the persistent buffer is defined there at this location model.model.normalize but model.model have children modules such as layers.
cc @ArthurZucker

Another solution would be to just use non persistant buffer all the time for these cases
-> This is the solution that we are going with for now. We can merge this PR if it makes sense later.

@HuggingFaceDocBuilderDev

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.

Copy link
Collaborator

@muellerzr muellerzr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, overall this looks fine to me.

Let's make a code comment here potentially, something like:

TODO: May have cpu and disk offload edge-cases.

@SunMarc SunMarc changed the title add all level buffer support when computing infer_auto_device_map [DO NOT MERGE] add all level buffer support when computing infer_auto_device_map Apr 12, 2024
Copy link

This issue has been automatically marked as stale because it has not had recent activity. If you think this still needs to be addressed please comment on this thread.

Please note that issues that do not follow the contributing guidelines are likely to be ignored.

@muellerzr muellerzr added enhancement New feature or request feature request Request for a new feature to be added to Accelerate wip Work in progress labels May 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request feature request Request for a new feature to be added to Accelerate wip Work in progress
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants