-
Notifications
You must be signed in to change notification settings - Fork 665
Fix crash in optimized log softmax #13953
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
Conversation
🔗 Helpful Links🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/executorch/13953
Note: Links to docs will display an error until the docs builds have been completed. ❌ 2 New FailuresAs of commit 53e3e2b with merge base 0eed262 ( NEW FAILURES - The following jobs have failed:
This comment was automatically generated by Dr. CI and updates every 15 minutes. |
This PR needs a
|
77d76f8
to
958b1c0
Compare
@mergennachin has imported this pull request. If you are a Meta employee, you can view this in D81703958. |
Summary: Fixes #13551 🐛 Problem The optimized log_softmax kernel crashed with fatal assertions when encountering unsupported double precision dtypes: F 00:00:00.005478 executorch:op_log_softmax.cpp:156] assert failed (false): Unhandled out dtype 7 ✅ Solution Replaced fatal ET_CHECK_MSG with graceful ET_KERNEL_CHECK error handling: Before: Program termination on unsupported dtypesAfter: Returns error codes, allows program to continue 📝 Changes kernels/optimized/cpu/op_log_softmax.cpp: - Converted log_softmax_wrapper to return bool success status - Replaced fatal assertions with graceful error returns - Added proper error state handling via ET_KERNEL_CHECK kernels/test/op_log_softmax_test.cpp: - Added DoubleCase test for comprehensive double precision validation - Implemented smart conditional testing: runs full validation on portable kernels, verifies graceful failure + skips on optimized kernels - Uses expect_failure() for CI-friendly error state handling 🎯 Results | Kernel | Double Precision Support | Behavior | CI Status | |-----------|--------------------------|-----------------------------|------------| | Portable | ✅ Full support | Test passes with validation | 🟢 PASSED | | Optimized | ❌ Not supported | Graceful error + skip | 🟢 SKIPPED | 🛡️ Benefits - No more crashes: Applications can handle unsupported operations gracefully - Better testability: Error conditions can now be tested and validated - CI compatibility: Green builds for both supported and unsupported operations - Regression protection: Prevents future reintroduction of fatal errors Testing: Verified on both portable and optimized kernel test suites ✅ Reviewed By: manuelcandales Differential Revision: D81703958 Pulled By: mergennachin
958b1c0
to
92fcf86
Compare
This pull request was exported from Phabricator. Differential Revision: D81703958 |
Summary: Fixes #13551 🐛 Problem The optimized log_softmax kernel crashed with fatal assertions when encountering unsupported double precision dtypes: F 00:00:00.005478 executorch:op_log_softmax.cpp:156] assert failed (false): Unhandled out dtype 7 ✅ Solution Replaced fatal ET_CHECK_MSG with graceful ET_KERNEL_CHECK error handling: Before: Program termination on unsupported dtypesAfter: Returns error codes, allows program to continue 📝 Changes kernels/optimized/cpu/op_log_softmax.cpp: - Converted log_softmax_wrapper to return bool success status - Replaced fatal assertions with graceful error returns - Added proper error state handling via ET_KERNEL_CHECK kernels/test/op_log_softmax_test.cpp: - Added DoubleCase test for comprehensive double precision validation - Implemented smart conditional testing: runs full validation on portable kernels, verifies graceful failure + skips on optimized kernels - Uses expect_failure() for CI-friendly error state handling 🎯 Results | Kernel | Double Precision Support | Behavior | CI Status | |-----------|--------------------------|-----------------------------|------------| | Portable | ✅ Full support | Test passes with validation | 🟢 PASSED | | Optimized | ❌ Not supported | Graceful error + skip | 🟢 SKIPPED | 🛡️ Benefits - No more crashes: Applications can handle unsupported operations gracefully - Better testability: Error conditions can now be tested and validated - CI compatibility: Green builds for both supported and unsupported operations - Regression protection: Prevents future reintroduction of fatal errors Testing: Verified on both portable and optimized kernel test suites ✅ Reviewed By: manuelcandales Differential Revision: D81703958 Pulled By: mergennachin
92fcf86
to
53e3e2b
Compare
This pull request was exported from Phabricator. Differential Revision: D81703958 |
Fixes #13551
🐛 Problem
The optimized log_softmax kernel crashed with fatal assertions when encountering unsupported double precision dtypes:
F 00:00:00.005478 executorch:op_log_softmax.cpp:156] assert failed (false): Unhandled out dtype 7
✅ Solution
Replaced fatal ET_CHECK_MSG with graceful ET_KERNEL_CHECK error handling:
Before: Program termination on unsupported dtypesAfter: Returns error codes, allows program to continue
📝 Changes
kernels/optimized/cpu/op_log_softmax.cpp:
kernels/test/op_log_softmax_test.cpp:
🎯 Results
🛡️ Benefits
Testing: Verified on both portable and optimized kernel test suites ✅