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

Update VITS modeling to enable ONNX export #28141

Merged
merged 3 commits into from
Jan 5, 2024

Conversation

echarlaix
Copy link
Collaborator

@echarlaix echarlaix commented Dec 19, 2023

This PR enables the ONNX export of VITS models in Optimum (huggingface/optimum#1607), currently the export is failing due to a cast operator added before the pow operator in the model graph, resulting in an issue during the concatenation of two values of different data type

cc @xenova

Copy link
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.

LGTM with the added suggestion.
For futur reference best practice is this only when it power of 2 ? does pow(x,2) work better (but is slower I think)

@xenova xenova self-requested a review December 20, 2023 12:28
Copy link
Contributor

@xenova xenova left a comment

Choose a reason for hiding this comment

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

Can confirm this fixes the ONNX exports (see here for a list I have converted). 👍

We can also update this for readability.

@echarlaix
Copy link
Collaborator Author

For futur reference best practice is this only when it power of 2 ? does pow(x,2) work better (but is slower I think)

In all cases it'll be casted to fp32 (no matter the exponent), using a multiplication instead removes this constraint, and should be "faster" (not that it'll have any impact here)

@echarlaix echarlaix merged commit 7226f3d into huggingface:main Jan 5, 2024
18 checks passed
@echarlaix echarlaix deleted the vits-onnx branch January 5, 2024 16:52
staghado pushed a commit to staghado/transformers that referenced this pull request Jan 15, 2024
* Update vits modeling for onnx export compatibility

* fix style

* Update src/transformers/models/vits/modeling_vits.py
wgifford pushed a commit to wgifford/transformers that referenced this pull request Jan 21, 2024
* Update vits modeling for onnx export compatibility

* fix style

* Update src/transformers/models/vits/modeling_vits.py
AjayP13 pushed a commit to AjayP13/transformers that referenced this pull request Jan 22, 2024
* Update vits modeling for onnx export compatibility

* fix style

* Update src/transformers/models/vits/modeling_vits.py
Bowen7 pushed a commit to Bowen7/transformers that referenced this pull request Mar 11, 2024
* Update vits modeling for onnx export compatibility

* fix style

* Update src/transformers/models/vits/modeling_vits.py
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.

3 participants