Skip to content
This repository was archived by the owner on Jun 3, 2025. It is now read-only.

Conversation

dbogunowicz
Copy link
Contributor

@dbogunowicz dbogunowicz commented Mar 21, 2022

Asana ticket: https://app.asana.com/0/1201735099598270/1201949554543506/f

There are some discrepancies that are probably harmless and expected. For instance, when comparing two pruned models, the sequences of last operations (before logits) differ slightly (this is probably due to how the newer PyTorch version handles the model-->onnx export pipeline).

The prediction head for the original onnx model ("before")
image

The prediction head for the retrieved onnx model ("after").
image

@review-notebook-app
Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

@dbogunowicz dbogunowicz requested a review from bfineran March 21, 2022 16:50
@dbogunowicz dbogunowicz self-assigned this Mar 21, 2022
Copy link
Contributor

@bfineran bfineran left a comment

Choose a reason for hiding this comment

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

looks like a good first pass, going to suggest that we refactor the tests into a class that can test parts of the pipeline and pass on (ie apply recipe -> load weights -> export ...)
(see https://docs.pytest.org/en/7.1.x/getting-started.html#group-multiple-tests-in-a-class)

@dbogunowicz dbogunowicz force-pushed the feature/damian/onnx_integration_tests branch from 27f868e to ebf6ea2 Compare March 24, 2022 15:57
@dbogunowicz dbogunowicz marked this pull request as ready for review March 24, 2022 16:07
@dbogunowicz dbogunowicz changed the title [WIP] Onnx integration tests for transformers. [Integration Testing for Quantized ONNX export] Export only testing Mar 24, 2022
@dbogunowicz dbogunowicz marked this pull request as draft March 25, 2022 13:04
@dbogunowicz dbogunowicz changed the title [Integration Testing for Quantized ONNX export] Export only testing [WIP] Integration Testing for Quantized ONNX export Mar 25, 2022
@dbogunowicz dbogunowicz marked this pull request as ready for review March 25, 2022 15:22
@dbogunowicz dbogunowicz requested a review from bfineran April 4, 2022 11:26
@dbogunowicz
Copy link
Contributor Author

dbogunowicz commented Apr 6, 2022

We do not have yet system in place to catch this error in CI/CD, but this PR will not be complete unless we also merge:
neuralmagic/sparsezoo#141

@bfineran
Copy link
Contributor

bfineran commented Apr 6, 2022

@dbogunowicz any reason not to use load_numpy_list? that should handle tar'd directories right?

@dbogunowicz
Copy link
Contributor Author

dbogunowicz commented Apr 7, 2022

@bfineran
Oh, only now I have realized that it load_numpy_list calls load_numpy_from_tar. You are absolutely right.

@dbogunowicz dbogunowicz force-pushed the feature/damian/onnx_integration_tests branch from 1c5291d to da182db Compare April 7, 2022 09:15
@dbogunowicz dbogunowicz changed the title [WIP] Integration Testing for Quantized ONNX export Integration Testing for Quantized ONNX export Apr 7, 2022
@dbogunowicz dbogunowicz requested review from a team, bfineran and markurtz and removed request for a team April 7, 2022 09:16
@dbogunowicz dbogunowicz requested a review from horheynm April 7, 2022 09:17
@dbogunowicz dbogunowicz added the 0.12 release Pull request pending for 0.12 release. label Apr 8, 2022
@KSGulin
Copy link
Contributor

KSGulin commented Apr 8, 2022

@dbogunowicz @bfineran Should transformers testing be added to GHA? It seems those tests are getting skipped, as they are a new category not yet included

@dbogunowicz dbogunowicz merged commit b95f153 into main Apr 8, 2022
@dbogunowicz dbogunowicz deleted the feature/damian/onnx_integration_tests branch April 8, 2022 11:23
dbogunowicz added a commit that referenced this pull request Apr 11, 2022
* Initial commit

* Refactor after Ben's comments

* Add testing for weights load and recipe application

* Fix style

Co-authored-by: bogunowicz@arrival.com <bogunowicz@arrival.com>
Co-authored-by: Benjamin Fineran <bfineran@users.noreply.github.com>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

0.12 release Pull request pending for 0.12 release.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants