Fix exception handling in torch._dynamo.utils.same and add corresponding test #125132

KevinHeeHee · 2024-04-29T02:08:31Z

Description:
This PR addresses an issue where the torch._dynamo.utils.same function swallows exceptions raised by torch.allclose without preserving the original exception information. It also adds a new test file to cover the scenario where an exception occurs during the torch.allclose check.

Changes:

Updated exception handling in torch._dynamo.utils.same:

Wrapped the torch.allclose check inside a try-except block.
In the except block, log the custom error message along with the original exception using logging.exception.
Re-raise the original exception to preserve the exception information.

Added a new test file test/dynamo/test_same_allclose.py:

Created a new test class TestSameAllclose that inherits from torch._dynamo.test_case.TestCase.
Defined two test methods:
- test_same_allclose_exception: Tests the scenario where torch.allclose raises a RuntimeError due to a size mismatch between input tensors. Asserts that the custom error message and original exception information are present in the raised exception.
- test_same_allclose_no_exception: Tests the case where no exception is expected, and torch.allclose should return True.
Imported the necessary modules: unittest, torch, and same from torch._dynamo.testing.

Benefits:

By improving the exception handling when an unexpected error occurs, this helps in debugging and understanding the root cause of the error
Adds test coverage and ensures the function behaves as expected
Ran testing to verify behavior

Fixes #111075
Please review this pull request and provide any feedback or suggestions for improvement.

Thank you!

cc @ezyang @msaroufim @bdhirsh @anijain2305 @chauhang @voznesenskym @penguinwu @EikanWang @jgong5 @Guobing-Chen @XiaobingSuper @zhuhaozhe @blzheng @wenzhe-nrv @jiayisunx @chenyang78 @kadeng

…oesn't swallow exceptions and maintains original error context

…rror messages are both not obscured in the case that an exception happens. Also added a test for when an exception is not raised for completeness

pytorch-bot · 2024-04-29T02:08:34Z

🔗 Helpful Links

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

📄 Preview Python docs built from this PR
📄 Preview C++ docs built from this PR
❓ Need help or want to give feedback on the CI? Visit the bot commands wiki or our office hours

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

❌ 13 New Failures, 5 Unrelated Failures

As of commit 427d9f2 with merge base e3b9b71 ():

NEW FAILURES - The following jobs have failed:

Lint / lintrunner-noclang / linux-job (gh)
>>> Lint for torch/_dynamo/utils.py:
pull / linux-focal-cuda12.1-py3.10-gcc9 / test (default, 1, 5, linux.4xlarge.nvidia.gpu) (gh)
RuntimeError: dynamo/test_same_allclose 1/1 failed
pull / linux-focal-cuda12.1-py3.10-gcc9-sm86 / test (default, 1, 5, linux.g5.4xlarge.nvidia.gpu) (gh)
RuntimeError: dynamo/test_same_allclose 1/1 failed
pull / linux-focal-py3.11-clang10 / test (crossref, 1, 2, linux.2xlarge) (gh)
RuntimeError: dynamo/test_same_allclose 1/1 failed
pull / linux-focal-py3.11-clang10 / test (dynamo, 1, 3, linux.2xlarge) (gh)
RuntimeError: dynamo/test_same_allclose 1/1 failed
pull / linux-focal-py3.12-clang10 / test (dynamo, 1, 3, linux.2xlarge) (gh)
RuntimeError: dynamo/test_same_allclose 1/1 failed
pull / linux-focal-py3.8-clang10 / test (crossref, 1, 2, linux.2xlarge) (gh)
RuntimeError: dynamo/test_same_allclose 1/1 failed
pull / linux-focal-py3.8-clang10 / test (dynamo, 1, 3, linux.2xlarge) (gh)
RuntimeError: dynamo/test_same_allclose 1/1 failed
pull / linux-jammy-py3.10-clang15-asan / test (default, 1, 6, linux.4xlarge) (gh)
RuntimeError: dynamo/test_same_allclose 1/1 failed
pull / linux-jammy-py3.8-gcc11 / test (default, 1, 3, linux.2xlarge) (gh)
RuntimeError: dynamo/test_same_allclose 1/1 failed
trunk / linux-focal-cuda12.1-py3.10-gcc9 / test (nogpu_AVX512, 1, 1, linux.2xlarge) (gh)
RuntimeError: dynamo/test_same_allclose 1/1 failed
trunk / linux-focal-cuda12.1-py3.10-gcc9 / test (nogpu_NO_AVX2, 1, 1, linux.2xlarge) (gh)
RuntimeError: dynamo/test_same_allclose 1/1 failed
trunk / macos-12-py3-arm64 / test (default, 1, 3, macos-m1-stable) (gh)
RuntimeError: dynamo/test_same_allclose 1/1 failed

FLAKY - The following jobs failed but were likely due to flakiness present on trunk:

linux-aarch64 / linux-jammy-aarch64-py3.10 / build (gh)
Process completed with exit code 126.
pull / linux-focal-py3.11-clang10 / test (default, 1, 3, linux.2xlarge) (gh)
test_cpp_extensions_open_device_registration.py::TestCppExtensionOpenRgistration::test_open_device_registration
pull / linux-focal-py3.12-clang10 / test (default, 1, 3, linux.2xlarge) (gh)
test_cpp_extensions_open_device_registration.py::TestCppExtensionOpenRgistration::test_open_device_registration
pull / linux-focal-py3.8-clang10 / test (default, 1, 3, linux.2xlarge) (gh)
test_cpp_extensions_open_device_registration.py::TestCppExtensionOpenRgistration::test_open_device_registration
trunk / win-vs2019-cpu-py3 / test (default, 1, 3, windows.4xlarge.nonephemeral) (gh)
test_cpp_extensions_open_device_registration.py::TestCppExtensionOpenRgistration::test_open_device_registration

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

linux-foundation-easycla · 2024-04-29T02:08:36Z

The committers listed above are authorized under a signed CLA.

✅ login: KevinHeeHee / name: Kevin He (b040072, 9b70967, 427d9f2, 0f96771)
✅ login: ezyang / name: Edward Z. Yang (f2aa622, 237e5d6)

ezyang

okey dokey

ezyang · 2024-04-29T13:34:39Z

@pytorchbot merge

pytorchmergebot · 2024-04-29T13:36:36Z

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

pytorchmergebot · 2024-04-29T13:52:11Z

Merge failed

Reason: 1 mandatory check(s) failed. The first few are:

Lint / lintrunner-noclang / linux-job

Dig deeper by viewing the failures on hud

Details for Dev Infra team

Raised by workflow job

Failing merge rule: Core Maintainers

…ue111075

ezyang · 2024-04-30T00:30:19Z

@pytorchbot merge

pytorchmergebot · 2024-04-30T00:32:49Z

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

pytorchmergebot · 2024-04-30T00:33:03Z

Merge failed

Reason: 3 jobs have failed, first few of them are: trunk / macos-12-py3-arm64 / test (default, 1, 3, macos-m1-stable), trunk / linux-focal-cuda12.1-py3.10-gcc9 / test (nogpu_AVX512, 1, 1, linux.2xlarge), trunk / linux-focal-cuda12.1-py3.10-gcc9 / test (nogpu_NO_AVX2, 1, 1, linux.2xlarge)

Details for Dev Infra team

Raised by workflow job

KevinHeeHee added 2 commits April 28, 2024 21:44

Changed the exception handling so that exception handling mechanism d…

b040072

…oesn't swallow exceptions and maintains original error context

Added test in dynamo directory to check that the original and final e…

9b70967

…rror messages are both not obscured in the case that an exception happens. Also added a test for when an exception is not raised for completeness

pytorch-bot bot added module: dynamo oncall: pt2 labels Apr 29, 2024

pytorchbot added the open source label Apr 29, 2024

ezyang added 2 commits April 29, 2024 09:33

Update utils.py

237e5d6

Update test_same_allclose.py

f2aa622

ezyang added the topic: not user facing topic category label Apr 29, 2024

ezyang approved these changes Apr 29, 2024

View reviewed changes

pytorch-bot bot added the ciflow/trunk Trigger trunk jobs on your pull request label Apr 29, 2024

pytorchmergebot added the merging label Apr 29, 2024

pytorchmergebot removed the merging label Apr 29, 2024

KevinHeeHee added 2 commits April 29, 2024 19:00

Fixed some linter issues pt1

0f96771

Merge branch 'issue111075' of github.com:KevinHeeHee/pytorch into iss…

427d9f2

…ue111075

pytorchmergebot added the merging label Apr 30, 2024

pytorchmergebot removed the merging label Apr 30, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix exception handling in torch._dynamo.utils.same and add corresponding test #125132

Fix exception handling in torch._dynamo.utils.same and add corresponding test #125132

KevinHeeHee commented Apr 29, 2024 •

edited by pytorch-bot bot

pytorch-bot bot commented Apr 29, 2024 •

edited

linux-foundation-easycla bot commented Apr 29, 2024 •

edited

ezyang left a comment

ezyang commented Apr 29, 2024

pytorchmergebot commented Apr 29, 2024

pytorchmergebot commented Apr 29, 2024

ezyang commented Apr 30, 2024

pytorchmergebot commented Apr 30, 2024

pytorchmergebot commented Apr 30, 2024

Fix exception handling in torch._dynamo.utils.same and add corresponding test #125132

Are you sure you want to change the base?

Fix exception handling in torch._dynamo.utils.same and add corresponding test #125132

Conversation

KevinHeeHee commented Apr 29, 2024 • edited by pytorch-bot bot

pytorch-bot bot commented Apr 29, 2024 • edited

🔗 Helpful Links

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

❌ 13 New Failures, 5 Unrelated Failures

linux-foundation-easycla bot commented Apr 29, 2024 • edited

ezyang left a comment

Choose a reason for hiding this comment

ezyang commented Apr 29, 2024

pytorchmergebot commented Apr 29, 2024

Merge started

pytorchmergebot commented Apr 29, 2024

Merge failed

ezyang commented Apr 30, 2024

pytorchmergebot commented Apr 30, 2024

Merge started

pytorchmergebot commented Apr 30, 2024

Merge failed

KevinHeeHee commented Apr 29, 2024 •

edited by pytorch-bot bot

pytorch-bot bot commented Apr 29, 2024 •

edited

linux-foundation-easycla bot commented Apr 29, 2024 •

edited