Skip to content

Conversation

@Cyrilvallez
Copy link
Member

@Cyrilvallez Cyrilvallez commented Nov 19, 2025

What does this PR do?

Follow up to #42264.
Since #42043, compute_module_sizes is also used in _init_infer_auto_device_map. However, there it expects to have all the modules inside, but also the leaves parameters and buffers, which was not the case. Because we return defaultdict instance, we would not have any KeyError but instead very nasty silent bugs because the values found would always be 0 (the default of the defaultdicts.

This PR fixes it by adding the option for adding them in the dict. Doing compute_module_sizes(model, only_modules=False) is now equivalent to accelerate.compute_module_sizes(model), which is not the case without the flag (leaves are not in the dict)

cc @SunMarc

@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
Member

@SunMarc SunMarc left a comment

Choose a reason for hiding this comment

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

Indeed, this was indeed strange that in this version of compute_module_sizes it returned two values. Thanks for fixing this nasty bug !

@Cyrilvallez Cyrilvallez merged commit 59dfc1d into main Nov 19, 2025
22 of 24 checks passed
@Cyrilvallez Cyrilvallez deleted the accelerate-integration branch November 19, 2025 17:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants