KEMBAR78
[modular] Use multi-processing + fix model import issue by Cyrilvallez · Pull Request #40481 · huggingface/transformers · GitHub
Skip to content

Conversation

@Cyrilvallez
Copy link
Member

@Cyrilvallez Cyrilvallez commented Aug 27, 2025

What does this PR do?

As per the title. Let's use multiprocessing by default on the converter, and it also fixes relative imports of the form from ...models.name.modeling_ import ... that caused the issue of on-conversion in #40431

cc @qubvel as well, only the checker was using mp until now -> high time for the converter to do it as well, as running all conversions locally is starting to take quite some time

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

Comment on lines -1229 to +1230
rf"(?:transformers\.models\.)|(?:\.\.)\w+\.({self.match_patterns})_.*", import_statement
rf"(?:transformers\.models\.)|(?:\.\.\.models\.)|(?:\.\.)\w+\.({self.match_patterns})_.*",
Copy link
Member Author

Choose a reason for hiding this comment

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

This was missing, which caused from ...models.name.modeling import issues in #40431

Copy link
Contributor

@qubvel qubvel left a comment

Choose a reason for hiding this comment

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

Thanks! Looks good

At some point, we might need to organize a topologically sorted queue with multiple workers if the dependency tree depth grows significantly (keeping the width low, e.g. 1-2). However, this is unlikely to happen if we reuse more modules from older models than the recent ones

@cyyever
Copy link
Contributor

cyyever commented Aug 27, 2025

Looks good, I will rebase it and check any remaining changes once merged.

@Cyrilvallez Cyrilvallez merged commit a3afebb into main Aug 27, 2025
15 checks passed
@Cyrilvallez Cyrilvallez deleted the modular-mp branch August 27, 2025 12:51
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