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

[ONNX] Run ONNX tests as part of standard run_test script #99215

Closed
wants to merge 8 commits into from

Conversation

BowenBao
Copy link
Collaborator

@BowenBao BowenBao commented Apr 15, 2023

Stack from ghstack (oldest at bottom):

🤖 Generated by Copilot at dcbf7e2

Summary

📝🧹🚩

This pull request improves the ONNX testing infrastructure in PyTorch by refactoring the test code, normalizing the scope names, adding a flag to run only the ONNX tests, and simplifying the test script.

To export PyTorch models to ONNX
We refactored some scripts and contexts
We used common_utils
And normalized the scopes
And added a flag to run the tests

Walkthrough

  • Simplify ./scripts/onnx/test.sh to use run_test.py with --onnx flag instead of pytest (link)
  • Remove onnx test from TESTS list in test/run_test.py (link). Replace with onnx_caffe2.
  • Add onnx/test_pytorch_onnx_onnxruntime_cuda and onnx/test_models tests to blocklisted_tests list in test/run_test.py (link)
  • Add ONNX_SERIAL_LIST list to test/run_test.py to specify ONNX tests that must run serially (link)
  • Add ONNX_TESTS list to test/run_test.py to store all ONNX tests (link)
  • Add --onnx flag to parse_args function in test/run_test.py to run only ONNX tests (link)
  • Include ONNX_SERIAL_LIST in must_serial function in test/run_test.py to run ONNX tests serially or parallelly based on memory usage (link)
  • Filter selected tests based on --onnx flag in get_selected_tests function in test/run_test.py to exclude non-ONNX tests (link)

Other minor changes to accommodate this change

  • Replace unittest module with common_utils.TestCase in test/onnx/dynamo/test_exporter_api.py (link, link, link, link)
  • Import TemporaryFileName class from common_utils in test/onnx/dynamo/test_exporter_api.py (link)
  • Use common_utils.TemporaryFileName instead of TemporaryFileName in TestDynamoExportAPI class in test/onnx/dynamo/test_exporter_api.py (link, link, link)
  • Use common_utils.run_tests instead of unittest.main in test/onnx/dynamo/test_exporter_api.py (link)
  • Add re module to test/onnx/test_utility_funs.py (link)
  • Add _remove_test_environment_prefix_from_scope_name function to test/onnx/test_utility_funs.py to normalize scope names of ONNX nodes (link)
  • Use _remove_test_environment_prefix_from_scope_name function to compare scope names of ONNX nodes in TestUtilityFuns class in test/onnx/test_utility_funs.py (link, link, link, link, link, link)

Fixes #98626

cc @seemethere @malfet @pytorch/pytorch-dev-infra

@pytorch-bot
Copy link

pytorch-bot bot commented Apr 15, 2023

🔗 Helpful Links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/99215

Note: Links to docs will display an error until the docs builds have been completed.

❗ 1 Active SEVs

There are 1 currently active SEVs. If your PR is affected, please view them below:

✅ No Failures

As of commit d540039:
💚 Looks good so far! There are no failures yet. 💚

This comment was automatically generated by Dr. CI and updates every 15 minutes.

@pytorch-bot pytorch-bot bot added topic: not user facing topic category labels Apr 15, 2023
BowenBao added a commit that referenced this pull request Apr 15, 2023
ghstack-source-id: b04dfd488df773762727e7e47facb3f0c469586b
Pull Request resolved: #99215
BowenBao added a commit that referenced this pull request Apr 15, 2023
ghstack-source-id: bbef3cb8813d06b3a9dc20c37e225a2323378f87
Pull Request resolved: #99215
…NX tests as part of standard run_test script"

[ghstack-poisoned]
BowenBao added a commit that referenced this pull request Apr 17, 2023
ghstack-source-id: d01565e09c45e3c121f2961c3369c2e9cb27cb76
Pull Request resolved: #99215
@BowenBao BowenBao added module: onnx Related to torch.onnx module: ci Related to continuous integration labels Apr 17, 2023
…ts as part of standard run_test script"

cc seemethere malfet pytorch/pytorch-dev-infra

[ghstack-poisoned]
BowenBao added a commit that referenced this pull request Apr 17, 2023
ghstack-source-id: 8370e47ae8650780d4bbeeef04eb627302821827
Pull Request resolved: #99215
…pytest on "[ONNX] Run ONNX tests as part of standard run_test script"

cc seemethere malfet pytorch/pytorch-dev-infra

[ghstack-poisoned]
BowenBao added a commit that referenced this pull request Apr 17, 2023
ghstack-source-id: da37590e8f873707b6018c71ed4d66753387be3e
Pull Request resolved: #99215
cc seemethere malfet pytorch/pytorch-dev-infra

[ghstack-poisoned]
BowenBao added a commit that referenced this pull request Apr 17, 2023
ghstack-source-id: 8473517a28316dc2611803a61286fa5821d84719
Pull Request resolved: #99215
…of standard run_test script"


<!--
copilot:all
-->
### <samp>🤖 Generated by Copilot at 8277fb8</samp>

### Summary
🧪🛠️🚀

<!--
1.  🧪 for simplifying the test script for onnx
2.  🛠️ for refactoring the test module for onnx exporter api
3.  🚀 for enabling more testing features and improving performance
-->
This pull request refactors and improves the onnx testing code in `pytorch/pytorch`. It uses the common_utils module to simplify and standardize the test cases, adds a helper function to normalize the scope names of onnx nodes, enables running onnx tests separately and in subtests with the `run_test.py` script, and simplifies the `scripts/onnx/test.sh` script.

> _To test onnx without much hassle_
> _The code uses `run_test.py` as a vessel_
> _It normalizes the scopes_
> _And refactors the tropes_
> _Of the exporter api and its tassel_

### Walkthrough
*  Simplify onnx test script by using `run_test.py` with `--onnx` flag ([link](https://github.com/pytorch/pytorch/pull/99215/files?diff=unified&w=0#diff-0017f5b22ae1329acb0f54af8d9811c9b6180a72dac70d7a5b89d7c23c958198L44-R46))
*  Remove unused import of `unittest` from onnx exporter api test module ([link](https://github.com/pytorch/pytorch/pull/99215/files?diff=unified&w=0#diff-4545f0c15c73ebe90a875e9bee6c5ca4b6b92fb1ed0ec5560d1568e0f6339d02L4))
*  Change base class of onnx exporter api test classes from `unittest.TestCase` to `common_utils.TestCase` to inherit useful methods and attributes ([link](https://github.com/pytorch/pytorch/pull/99215/files?diff=unified&w=0#diff-4545f0c15c73ebe90a875e9bee6c5ca4b6b92fb1ed0ec5560d1568e0f6339d02L19-R18), [link](https://github.com/pytorch/pytorch/pull/99215/files?diff=unified&w=0#diff-4545f0c15c73ebe90a875e9bee6c5ca4b6b92fb1ed0ec5560d1568e0f6339d02L29-R28), [link](https://github.com/pytorch/pytorch/pull/99215/files?diff=unified&w=0#diff-4545f0c15c73ebe90a875e9bee6c5ca4b6b92fb1ed0ec5560d1568e0f6339d02L71-R70), [link](https://github.com/pytorch/pytorch/pull/99215/files?diff=unified&w=0#diff-4545f0c15c73ebe90a875e9bee6c5ca4b6b92fb1ed0ec5560d1568e0f6339d02L147-R146))
*  Change import and usage of `TemporaryFileName` from global to class attribute of `common_utils.TestCase` to avoid name clashes and improve consistency ([link](https://github.com/pytorch/pytorch/pull/99215/files?diff=unified&w=0#diff-4545f0c15c73ebe90a875e9bee6c5ca4b6b92fb1ed0ec5560d1568e0f6339d02L92-R91), [link](https://github.com/pytorch/pytorch/pull/99215/files?diff=unified&w=0#diff-4545f0c15c73ebe90a875e9bee6c5ca4b6b92fb1ed0ec5560d1568e0f6339d02L110-R109), [link](https://github.com/pytorch/pytorch/pull/99215/files?diff=unified&w=0#diff-4545f0c15c73ebe90a875e9bee6c5ca4b6b92fb1ed0ec5560d1568e0f6339d02L129-R128))
*  Change main entry point of onnx exporter api test module from `unittest.main` to `common_utils.run_tests` to make it compatible with `run_test.py` script ([link](https://github.com/pytorch/pytorch/pull/99215/files?diff=unified&w=0#diff-4545f0c15c73ebe90a875e9bee6c5ca4b6b92fb1ed0ec5560d1568e0f6339d02L155-R154))
*  Add helper function to remove test environment prefix from scope name of onnx nodes in `test_utility_funs.py` to make scope name comparisons more robust and consistent ([link](https://github.com/pytorch/pytorch/pull/99215/files?diff=unified&w=0#diff-da71d2c81c9dc7ac0c47ff086fded82e4edcb67ba0cd3d8b5c983d7467343bc7R31-R81))
*  Move and rename nested classes `_N` and `_M` from `test_node_scope` function to top level of `test_utility_funs.py` to fix class names for scope name normalization ([link](https://github.com/pytorch/pytorch/pull/99215/files?diff=unified&w=0#diff-da71d2c81c9dc7ac0c47ff086fded82e4edcb67ba0cd3d8b5c983d7467343bc7L1067-R1130))
*  Use helper function to remove test environment prefix from scope name of onnx nodes in `test_node_scope` function and update expected scope names ([link](https://github.com/pytorch/pytorch/pull/99215/files?diff=unified&w=0#diff-da71d2c81c9dc7ac0c47ff086fded82e4edcb67ba0cd3d8b5c983d7467343bc7L1119-R1149))
*  Update expected layer scope name to use new class name `_M` in `test_scope_of_constants_when_combined_by_cse_pass` and `test_scope_of_nodes_when_combined_by_cse_pass` functions ([link](https://github.com/pytorch/pytorch/pull/99215/files?diff=unified&w=0#diff-da71d2c81c9dc7ac0c47ff086fded82e4edcb67ba0cd3d8b5c983d7467343bc7L1172-R1186), [link](https://github.com/pytorch/pytorch/pull/99215/files?diff=unified&w=0#diff-da71d2c81c9dc7ac0c47ff086fded82e4edcb67ba0cd3d8b5c983d7467343bc7L1222-R1236))
*  Replace `onnx` test with `onnx_caffe2` test in `run_test.py` script to reflect test split ([link](https://github.com/pytorch/pytorch/pull/99215/files?diff=unified&w=0#diff-e72503c9e3e8766e2d1bacf3fad7b88aa166e0e90a7e103e7df99357a35df8d7L127-R127))
*  Add two new onnx subtests to `run_test.py` script that were previously excluded due to high memory usage or long running time ([link](https://github.com/pytorch/pytorch/pull/99215/files?diff=unified&w=0#diff-e72503c9e3e8766e2d1bacf3fad7b88aa166e0e90a7e103e7df99357a35df8d7R154-R155))
*  Add list of onnx subtests that cannot run in parallel due to high memory usage and run them serially when `--onnx` flag is used ([link](https://github.com/pytorch/pytorch/pull/99215/files?diff=unified&w=0#diff-e72503c9e3e8766e2d1bacf3fad7b88aa166e0e90a7e103e7df99357a35df8d7R296-R301), [link](https://github.com/pytorch/pytorch/pull/99215/files?diff=unified&w=0#diff-e72503c9e3e8766e2d1bacf3fad7b88aa166e0e90a7e103e7df99357a35df8d7R1120))
*  Add list of onnx tests that includes all subtests with onnx prefix and use it to filter selected tests based on `--onnx` flag ([link](https://github.com/pytorch/pytorch/pull/99215/files?diff=unified&w=0#diff-e72503c9e3e8766e2d1bacf3fad7b88aa166e0e90a7e103e7df99357a35df8d7R370), [link](https://github.com/pytorch/pytorch/pull/99215/files?diff=unified&w=0#diff-e72503c9e3e8766e2d1bacf3fad7b88aa166e0e90a7e103e7df99357a35df8d7R1158-R1165))
*  Add `--onnx` flag to `run_test.py` script to allow user to only run onnx tests or exclude them ([link](https://github.com/pytorch/pytorch/pull/99215/files?diff=unified&w=0#diff-e72503c9e3e8766e2d1bacf3fad7b88aa166e0e90a7e103e7df99357a35df8d7R920-R928))



cc seemethere malfet pytorch/pytorch-dev-infra

[ghstack-poisoned]
BowenBao added a commit that referenced this pull request Apr 17, 2023
ghstack-source-id: 82ab204175e7a0dcd81405ad4d9913fef481a7c1
Pull Request resolved: #99215
@BowenBao BowenBao marked this pull request as ready for review April 18, 2023 00:21
@BowenBao BowenBao requested review from a team and abock as code owners April 18, 2023 00:21
Copy link
Contributor

@huydhn huydhn 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 for making this happen.

… tests as part of standard run_test script"


<!--
copilot:all
-->
### <samp>🤖 Generated by Copilot at dcbf7e2</samp>

### Summary
📝🧹🚩

<!--
1.  📝 for simplifying the `./scripts/onnx/test.sh` script
2.  🧹 for refactoring the `test/onnx/dynamo/test_exporter_api.py` file
3.  🚩 for adding the `--onnx` flag to `test/run_test.py` and updating the `TESTS` list
-->
This pull request improves the ONNX testing infrastructure in PyTorch by refactoring the test code, normalizing the scope names, adding a flag to run only the ONNX tests, and simplifying the test script.

> _To export PyTorch models to ONNX_
> _We refactored some scripts and contexts_
> _We used `common_utils`_
> _And normalized the scopes_
> _And added a flag to run the tests_

### Walkthrough
*  Simplify `./scripts/onnx/test.sh` to use `run_test.py` with `--onnx` flag instead of `pytest` ([link](https://github.com/pytorch/pytorch/pull/99215/files?diff=unified&w=0#diff-0017f5b22ae1329acb0f54af8d9811c9b6180a72dac70d7a5b89d7c23c958198L44-R46))
*  Remove `onnx` test from `TESTS` list in `test/run_test.py` ([link](https://github.com/pytorch/pytorch/pull/99215/files?diff=unified&w=0#diff-e72503c9e3e8766e2d1bacf3fad7b88aa166e0e90a7e103e7df99357a35df8d7L127-R127)). Replace with `onnx_caffe2`.
*  Add `onnx/test_pytorch_onnx_onnxruntime_cuda` and `onnx/test_models` tests to `blocklisted_tests` list in `test/run_test.py` ([link](https://github.com/pytorch/pytorch/pull/99215/files?diff=unified&w=0#diff-e72503c9e3e8766e2d1bacf3fad7b88aa166e0e90a7e103e7df99357a35df8d7R154-R155))
*  Add `ONNX_SERIAL_LIST` list to `test/run_test.py` to specify ONNX tests that must run serially ([link](https://github.com/pytorch/pytorch/pull/99215/files?diff=unified&w=0#diff-e72503c9e3e8766e2d1bacf3fad7b88aa166e0e90a7e103e7df99357a35df8d7R296-R301))
*  Add `ONNX_TESTS` list to `test/run_test.py` to store all ONNX tests ([link](https://github.com/pytorch/pytorch/pull/99215/files?diff=unified&w=0#diff-e72503c9e3e8766e2d1bacf3fad7b88aa166e0e90a7e103e7df99357a35df8d7R370))
*  Add `--onnx` flag to `parse_args` function in `test/run_test.py` to run only ONNX tests ([link](https://github.com/pytorch/pytorch/pull/99215/files?diff=unified&w=0#diff-e72503c9e3e8766e2d1bacf3fad7b88aa166e0e90a7e103e7df99357a35df8d7R920-R928))
*  Include `ONNX_SERIAL_LIST` in `must_serial` function in `test/run_test.py` to run ONNX tests serially or parallelly based on memory usage ([link](https://github.com/pytorch/pytorch/pull/99215/files?diff=unified&w=0#diff-e72503c9e3e8766e2d1bacf3fad7b88aa166e0e90a7e103e7df99357a35df8d7R1120))
*  Filter selected tests based on `--onnx` flag in `get_selected_tests` function in `test/run_test.py` to exclude non-ONNX tests ([link](https://github.com/pytorch/pytorch/pull/99215/files?diff=unified&w=0#diff-e72503c9e3e8766e2d1bacf3fad7b88aa166e0e90a7e103e7df99357a35df8d7R1158-R1165))

### Other minor changes to accommodate this change
*  Replace `unittest` module with `common_utils.TestCase` in `test/onnx/dynamo/test_exporter_api.py` ([link](https://github.com/pytorch/pytorch/pull/99215/files?diff=unified&w=0#diff-4545f0c15c73ebe90a875e9bee6c5ca4b6b92fb1ed0ec5560d1568e0f6339d02L4), [link](https://github.com/pytorch/pytorch/pull/99215/files?diff=unified&w=0#diff-4545f0c15c73ebe90a875e9bee6c5ca4b6b92fb1ed0ec5560d1568e0f6339d02L29-R28), [link](https://github.com/pytorch/pytorch/pull/99215/files?diff=unified&w=0#diff-4545f0c15c73ebe90a875e9bee6c5ca4b6b92fb1ed0ec5560d1568e0f6339d02L71-R70), [link](https://github.com/pytorch/pytorch/pull/99215/files?diff=unified&w=0#diff-4545f0c15c73ebe90a875e9bee6c5ca4b6b92fb1ed0ec5560d1568e0f6339d02L147-R146))
*  Import `TemporaryFileName` class from `common_utils` in `test/onnx/dynamo/test_exporter_api.py` ([link](https://github.com/pytorch/pytorch/pull/99215/files?diff=unified&w=0#diff-4545f0c15c73ebe90a875e9bee6c5ca4b6b92fb1ed0ec5560d1568e0f6339d02L19-R18))
*  Use `common_utils.TemporaryFileName` instead of `TemporaryFileName` in `TestDynamoExportAPI` class in `test/onnx/dynamo/test_exporter_api.py` ([link](https://github.com/pytorch/pytorch/pull/99215/files?diff=unified&w=0#diff-4545f0c15c73ebe90a875e9bee6c5ca4b6b92fb1ed0ec5560d1568e0f6339d02L92-R91), [link](https://github.com/pytorch/pytorch/pull/99215/files?diff=unified&w=0#diff-4545f0c15c73ebe90a875e9bee6c5ca4b6b92fb1ed0ec5560d1568e0f6339d02L110-R109), [link](https://github.com/pytorch/pytorch/pull/99215/files?diff=unified&w=0#diff-4545f0c15c73ebe90a875e9bee6c5ca4b6b92fb1ed0ec5560d1568e0f6339d02L129-R128))
*  Use `common_utils.run_tests` instead of `unittest.main` in `test/onnx/dynamo/test_exporter_api.py` ([link](https://github.com/pytorch/pytorch/pull/99215/files?diff=unified&w=0#diff-4545f0c15c73ebe90a875e9bee6c5ca4b6b92fb1ed0ec5560d1568e0f6339d02L155-R154))
*  Add `re` module to `test/onnx/test_utility_funs.py` ([link](https://github.com/pytorch/pytorch/pull/99215/files?diff=unified&w=0#diff-da71d2c81c9dc7ac0c47ff086fded82e4edcb67ba0cd3d8b5c983d7467343bc7R6))
*  Add `_remove_test_environment_prefix_from_scope_name` function to `test/onnx/test_utility_funs.py` to normalize scope names of ONNX nodes ([link](https://github.com/pytorch/pytorch/pull/99215/files?diff=unified&w=0#diff-da71d2c81c9dc7ac0c47ff086fded82e4edcb67ba0cd3d8b5c983d7467343bc7R32-R58))
*  Use `_remove_test_environment_prefix_from_scope_name` function to compare scope names of ONNX nodes in `TestUtilityFuns` class in `test/onnx/test_utility_funs.py` ([link](https://github.com/pytorch/pytorch/pull/99215/files?diff=unified&w=0#diff-da71d2c81c9dc7ac0c47ff086fded82e4edcb67ba0cd3d8b5c983d7467343bc7L1099-R1133), [link](https://github.com/pytorch/pytorch/pull/99215/files?diff=unified&w=0#diff-da71d2c81c9dc7ac0c47ff086fded82e4edcb67ba0cd3d8b5c983d7467343bc7L1119-R1152), [link](https://github.com/pytorch/pytorch/pull/99215/files?diff=unified&w=0#diff-da71d2c81c9dc7ac0c47ff086fded82e4edcb67ba0cd3d8b5c983d7467343bc7L1170-R1188), [link](https://github.com/pytorch/pytorch/pull/99215/files?diff=unified&w=0#diff-da71d2c81c9dc7ac0c47ff086fded82e4edcb67ba0cd3d8b5c983d7467343bc7L1181-R1199), [link](https://github.com/pytorch/pytorch/pull/99215/files?diff=unified&w=0#diff-da71d2c81c9dc7ac0c47ff086fded82e4edcb67ba0cd3d8b5c983d7467343bc7L1220-R1239), [link](https://github.com/pytorch/pytorch/pull/99215/files?diff=unified&w=0#diff-da71d2c81c9dc7ac0c47ff086fded82e4edcb67ba0cd3d8b5c983d7467343bc7L1235-R1258))


cc seemethere malfet pytorch/pytorch-dev-infra

[ghstack-poisoned]
BowenBao added a commit that referenced this pull request Apr 18, 2023
ghstack-source-id: 98c8f7d77f15d37c1fb3f57efbd30219e21a22e7
Pull Request resolved: #99215
@BowenBao
Copy link
Collaborator Author

@huydhn thanks for bringing this to our attention! By the way, do you have some wiki/docs for how to utilize the features of run_test such as test skips etc? Would love to share them with my team.

@huydhn
Copy link
Contributor

huydhn commented Apr 18, 2023

@huydhn thanks for bringing this to our attention! By the way, do you have some wiki/docs for how to utilize the features of run_test such as test skips etc? Would love to share them with my team.

cc @clee2000 Do you know if there is a good doc about run_test somewhere that we can share? I guess we really need to create one if there is none.

@BowenBao
Copy link
Collaborator Author

@pytorchbot merge

@pytorch-bot pytorch-bot bot added the ciflow/trunk Trigger trunk jobs on your pull request label Apr 18, 2023
@pytorchmergebot
Copy link
Collaborator

Merge started

Your change will be merged once all checks pass (ETA 0-4 Hours).

Learn more about merging in the wiki.

Questions? Feedback? Please reach out to the PyTorch DevX Team

Advanced Debugging
Check the merge workflow status
here

@BowenBao
Copy link
Collaborator Author

@pytorchbot merge

@pytorchmergebot
Copy link
Collaborator

Merge started

Your change will be merged once all checks pass (ETA 0-4 Hours).

Learn more about merging in the wiki.

Questions? Feedback? Please reach out to the PyTorch DevX Team

Advanced Debugging
Check the merge workflow status
here

@facebook-github-bot facebook-github-bot deleted the gh/BowenBao/232/head branch June 8, 2023 14:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ciflow/trunk Trigger trunk jobs on your pull request Merged module: ci Related to continuous integration module: onnx Related to torch.onnx open source topic: not user facing topic category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants