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

Support for custom input shapes in exporters onnx #575

Merged

Conversation

fxmarty
Copy link
Contributor

@fxmarty fxmarty commented Dec 12, 2022

This will be mainly useful for testing, but could also solve temporarily issues on models where the export is dependent on the input shape (for example longformer (but it's actually a bug in PyTorch), and longt5.

Related issues are linked in #503
This PR follows #522

Edit: let's wait for #573 to be merged to clean up

Before submitting

  • Did you write any new necessary tests?

@HuggingFaceDocBuilderDev
Copy link

HuggingFaceDocBuilderDev commented Dec 12, 2022

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

@fxmarty fxmarty marked this pull request as ready for review December 14, 2022 11:11
Copy link
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.

Thanks @fxmarty!!
Just left a couple of nits.

optimum/exporters/onnx/convert.py Outdated Show resolved Hide resolved
tests/exporters/test_onnx_export.py Outdated Show resolved Hide resolved
@fxmarty
Copy link
Contributor Author

fxmarty commented Dec 16, 2022

Let's wait for the stable diffusion PR to be merged before this.

Copy link
Member

@michaelbenayoun michaelbenayoun left a comment

Choose a reason for hiding this comment

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

Left a few comments, but besides that it looks great, thanks @fxmarty !

@@ -31,4 +31,4 @@ jobs:
- name: Test with unittest
working-directory: tests
run: |
RUN_SLOW=1 pytest exporters --durations=0
pytest exporters -s --durations=0
Copy link
Member

Choose a reason for hiding this comment

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

Are you sure you want to output everything in the log?

Copy link
Contributor Author

@fxmarty fxmarty Dec 16, 2022

Choose a reason for hiding this comment

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

yes, why not?

optimum/exporters/onnx/__main__.py Outdated Show resolved Hide resolved
optimum/exporters/onnx/__main__.py Outdated Show resolved Hide resolved
optimum/exporters/onnx/convert.py Outdated Show resolved Hide resolved
Copy link
Member

@michaelbenayoun michaelbenayoun left a comment

Choose a reason for hiding this comment

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

LGTM!

optimum/exporters/onnx/__main__.py Outdated Show resolved Hide resolved
optimum/exporters/onnx/__main__.py Outdated Show resolved Hide resolved
optimum/utils/testing_utils.py Outdated Show resolved Hide resolved
@fxmarty fxmarty merged commit c3914e9 into huggingface:main Dec 21, 2022
@HuggingFaceDocBuilderDev

The docs for this PR live here. All of your documentation changes will be reflected on that endpoint.

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