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

Adds GroupViT to models exportable with ONNX #18628

Merged
merged 2 commits into from
Aug 30, 2022

Conversation

unography
Copy link
Contributor

Adds GroupViT to models exportable with ONNX

@HuggingFaceDocBuilderDev
Copy link

HuggingFaceDocBuilderDev commented Aug 15, 2022

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

@unography
Copy link
Contributor Author

@regisss hopefully you don't have to make any changes this time to make the tests pass!

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.

LGTM, you are on fire @unography 🔥
Tests passed.

@regisss
Copy link
Contributor

regisss commented Aug 15, 2022

Pinging @sgugger for final approval

@LysandreJik
Copy link
Member

Feel free to merge if you approve @lewtun

@regisss
Copy link
Contributor

regisss commented Aug 24, 2022

@lewtun Can you take a quick look at this PR and merge it when you approve? 🙂

Copy link
Member

@lewtun lewtun left a comment

Choose a reason for hiding this comment

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

Thanks for adding this model to the ONNX exporter @unography - as in the OWLViT case, I just have a small comment about the shape of pixel_values. Otherwise this looks great!

return OrderedDict(
[
("input_ids", {0: "batch", 1: "sequence"}),
("pixel_values", {0: "batch"}),
Copy link
Member

Choose a reason for hiding this comment

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

Same comment as in the OWLViT PR: should this be

Suggested change
("pixel_values", {0: "batch"}),
("pixel_values", {0: "batch", 1: "num_channels", 2: "height", 3: "width"} }),

Copy link
Contributor Author

Choose a reason for hiding this comment

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

sure, added the change

Copy link
Member

@lewtun lewtun left a comment

Choose a reason for hiding this comment

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

Thanks a lot for iterating on the dynamic shapes @unography !

This LGTM, so gently pinging @sgugger for final approval

Copy link
Member

@LysandreJik LysandreJik left a comment

Choose a reason for hiding this comment

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

LGTM, thank you @unography!

@LysandreJik LysandreJik merged commit 220da3b into huggingface:main Aug 30, 2022
@unography unography deleted the groupvit_onnx branch August 30, 2022 14:03
oneraghavan pushed a commit to oneraghavan/transformers that referenced this pull request Sep 26, 2022
* groupvit to onnx

* dynamic shape for pixel values dim
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.

5 participants