Skip to content

Add Model loading from subfolders similar to transformers#443

Merged
fxmarty merged 4 commits into
huggingface:mainfrom
fxmarty:support-subfolder-onnx
Nov 4, 2022
Merged

Add Model loading from subfolders similar to transformers#443
fxmarty merged 4 commits into
huggingface:mainfrom
fxmarty:support-subfolder-onnx

Conversation

@fxmarty
Copy link
Copy Markdown
Contributor

@fxmarty fxmarty commented Nov 2, 2022

What does this PR do?

Allow to pass the argument subfolder to ORTModel.from_pretrained() in a similar fashion to transformers PreTrainedModel. It allows to host several models (e.g. optimized) in the same model repository.

The preprocessor is expected to be loaded from the parent directory.

Fixes #308

Before submitting

  • Did you make sure to update the documentation with your changes?
  • Did you write any new necessary tests?

@fxmarty fxmarty marked this pull request as ready for review November 2, 2022 12:31
@HuggingFaceDocBuilderDev
Copy link
Copy Markdown

HuggingFaceDocBuilderDev commented Nov 2, 2022

The documentation is not available anymore as the PR was closed or merged.

Copy link
Copy Markdown
Contributor

@regisss regisss left a comment

Choose a reason for hiding this comment

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

LGTM @fxmarty!

task = cls.export_feature
else:
task = HfApi().model_info(model_id, revision=revision).pipeline_tag
task = HfApi().model_info(model_id, revision=revision).pipeline_tag # FIXME: load from subfolder?
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Out of curiosity, the FIXME comment means that we are not sure if this can be done with HfApi?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

One of the issue is that the ONNX export in transformers relies on preprocessors to generate dummy inputs, hence we use preprocessor = get_preprocessor(model_id) that has no option to specify a subfolder. We would need to do a PR in transformers for that. Therefore, the tokenizer must be at the top level of the repo, as well as the config.json that may be required in AutoTokenizer to find the right class.

I changed a bit the code so that we don't need to expect a config.json in the subfolder, we fall back to using the one at the top level if there is not in the subfolder. Does it sound good to you?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Yes sounds good! In that case, maybe we could log a warning saying that the config was not found in the given subfolder and that we are looking for it in the parent folder.

@fxmarty
Copy link
Copy Markdown
Contributor Author

fxmarty commented Nov 3, 2022

Actually this could be useful if we want to save ONNX models in subfolders on the Hub? cc @JingyaHuang

@fxmarty fxmarty merged commit 205d29e into huggingface:main Nov 4, 2022
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.

Add Model loading from subfolders similar to transformers

3 participants