Skip to content

[modular] allow multiple modular files in the same model folder#36287

Open
Cyrilvallez wants to merge 1 commit into
mainfrom
multiple-modular
Open

[modular] allow multiple modular files in the same model folder#36287
Cyrilvallez wants to merge 1 commit into
mainfrom
multiple-modular

Conversation

@Cyrilvallez
Copy link
Copy Markdown
Member

What does this PR do?

As per the title. It fixes the issue mentioned in #36261. There are 2 small issues:

  • we were relying on the folder name to get the model name -> of course this fails if several modular files are in the same folder, so we now parse the full filename instead
  • Small prefix issue, in case the two models (the new one, and the one imported) have a common name pattern (i.e. in this case, rt_detr_resnet and d_fine_resnet both end with resnet, the prefix matching would lead to inconsistencies

@HuggingFaceDocBuilderDev
Copy link
Copy Markdown

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
Copy Markdown
Collaborator

@ArthurZucker ArthurZucker left a comment

Choose a reason for hiding this comment

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

Hey! I don't think we should allow this! Thanks for working on it but it will be a bit against our philO!

@VladOS95-cyber
Copy link
Copy Markdown
Contributor

VladOS95-cyber commented Feb 26, 2025

Hi @ArthurZucker! What do you propose to do in this situation then? For example, in case of D-FINE, we created resnet and main models modulars in the same folder like it is done for RT-DETR, but there, resnet is implemented from scratch and we are reusing it in D-FINE implementation that leads to some problems with modular conversion script which @Cyrilvallez is trying to resolve here.

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