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

Nystromformer ONNX export #728

Merged
merged 13 commits into from
Jan 31, 2023
Merged

Nystromformer ONNX export #728

merged 13 commits into from
Jan 31, 2023

Conversation

whr778
Copy link
Contributor

@whr778 whr778 commented Jan 28, 2023

This PR implements ONNX export functionality for Nystromformer models.

Fixes # (727)

In addition to running the test cases, I exported a locally built Nystromformer 2048 input sequence model to ONNX and ran prediction. I verified the prediction output.

I decided to use the uw-madison/nystromformer-1024 for testing and not the hf-internal-testing/tiny-random-NystromformerModel... as the inputs are wrong in the hf-internal-testing model.

Before submitting

  • This PR fixes a typo or improves the docs (you can dismiss the other checks if that's the case).
  • Did you make sure to update the documentation with your changes?
  • Did you write any new necessary tests?

@HuggingFaceDocBuilderDev
Copy link

HuggingFaceDocBuilderDev commented Jan 30, 2023

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

Copy link
Contributor

@fxmarty fxmarty left a comment

Choose a reason for hiding this comment

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

Thank you for your contribution, very clean! Just to understand, what is the issue with hf-internal-testing/tiny-random-NystromformerModel with the ONNX Runtime integration?

optimum/onnxruntime/utils.py Outdated Show resolved Hide resolved
whr778 and others added 2 commits January 30, 2023 11:27
Co-authored-by: fxmarty <9808326+fxmarty@users.noreply.github.com>
Co-authored-by: fxmarty <9808326+fxmarty@users.noreply.github.com>
@whr778
Copy link
Contributor Author

whr778 commented Jan 30, 2023

I ran the onnx test suite and was getting an error that the inputs were incorrect. I went back and verified that the coded inputs were correct, and then changed the model to the reference model from UW, and the test suite passed with no issues. I did not investigate the hf-internal-testing/tiny-random-NystromformerModel any further. I concluded there must be an issue with how it was created.

@fxmarty
Copy link
Contributor

fxmarty commented Jan 31, 2023

I tested locally with "nystromformer": "hf-internal-testing/tiny-random-NystromformerModel" and pytest tests/onnxruntime/test_modeling.py -k "nystromformer" --exitfirst -s and the tests pass.

Could you have a try again with the tiny model? It avoids to slow down too much the tests.

@whr778
Copy link
Contributor Author

whr778 commented Jan 31, 2023 via email

@whr778
Copy link
Contributor Author

whr778 commented Jan 31, 2023 via email

@whr778
Copy link
Contributor Author

whr778 commented Jan 31, 2023

@fxmarty changes are pushed. Thanks for the help!

@fxmarty
Copy link
Contributor

fxmarty commented Jan 31, 2023

Thanks great! The git history seems broken (see the diff), could you try to fix it? Not sure what's wrong, I already had the issue using git rebase.

Maybe git merge main would do (from upstream main).

@whr778
Copy link
Contributor Author

whr778 commented Jan 31, 2023

@fxmarty I think I have the git history fixed. I also ran make style. What appears to have happened is I inadvertently hit the sync fork button on the forked optimum repository web page.
I fixed it with a git merge upstream/main. E.g.

 1039  make style
 1040  git status
 1041  history | grep git
 1042  git fetch upstream
 1043  git merge upstream/main
 1044  git status
 1045  git push -u origin onnx-export-nystromformer

Sorry about the issues.

@fxmarty
Copy link
Contributor

fxmarty commented Jan 31, 2023

It's all good now, the failing test is unrelated. Thanks for the addition!

@fxmarty fxmarty merged commit 2633542 into huggingface:main Jan 31, 2023
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