[Eager] Fix trigger condition for Tensor reflected methods #73833
Conversation
| 你的PR提交成功，感谢你对开源项目的贡献! |
HydrogenSulfate force-pushed the fix_eager_add_and_mul_exception branch 2 times, most recently from 823a637 to 88b49ab Compare HydrogenSulfate force-pushed the fix_eager_add_and_mul_exception branch from 88b49ab to dddfc7b Compare There was a problem hiding this comment.
Pull Request Overview
This PR restores proper exception reporting for tensor reflected methods by moving the NotImplemented fallback into individual PyBind operator implementations and updating Python-side tests to validate multiple exception scenarios and dygraph behavior.
- Relocated reflected-method trigger logic from a global macro into each
__op__method so original exceptions surface correctly before falling back toNotImplemented - Updated tests to use a
dygraph_guardcontext and added new reflected-operation tests for custom tensors - Removed the now-unused global
EAGER_CATCH_AND_THROW_RETURN_NOT_IMPLEMENTEDmacro
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| test/legacy_test/test_math_op_patch.py | Swapped to dygraph_guard in tests, added reflected‐op tests, and updated CustomTensor methods |
| paddle/fluid/pybind/exception.h | Removed obsolete EAGER_CATCH_AND_THROW_RETURN_NOT_IMPLEMENTED macro |
| paddle/fluid/pybind/eager_math_op_patch.cc | Inlined per-operator fallback logic and removed macro usage |
Comments suppressed due to low confidence (1)
test/legacy_test/test_math_op_patch.py:669
- Re-adding the
@unittest.skipIf(paddle.device.is_compiled_with_xpu(), ...)decorator will prevent failures on XPU, where custom tensor operations aren't supported.
# @unittest.skipIf( | value = CastPyArg2Scalar(other_obj, "__add__", 0); | ||
| } catch (...) { | ||
| PyObject* radd = PyObject_GetAttrString(other_obj, "__radd__"); | ||
| if (radd) { |
There was a problem hiding this comment.
If PyObject_GetAttrString fails, it leaves a pending Python error. Call PyErr_Clear() when radd is nullptr to avoid a stale AttributeError in the interpreter state.
| if (radd) { | |
| if (!radd) { | |
| PyErr_Clear(); // Clear the Python error state if PyObject_GetAttrString fails | |
| } else { |
| try { | ||
| value = CastPyArg2Scalar(other_obj, "__add__", 0); | ||
| } catch (...) { | ||
| PyObject* radd = PyObject_GetAttrString(other_obj, "__radd__"); | ||
| if (radd) { | ||
| bool has_callable_radd = PyCallable_Check(radd); | ||
| Py_DECREF(radd); | ||
| if (has_callable_radd) { | ||
| Py_INCREF(Py_NotImplemented); | ||
| return Py_NotImplemented; | ||
| } | ||
| } | ||
| throw; | ||
| } |
There was a problem hiding this comment.
[nitpick] The reflected-method fallback pattern is repeated across operators; consider extracting this into a helper function to reduce duplication and improve readability.
| try { | |
| value = CastPyArg2Scalar(other_obj, "__add__", 0); | |
| } catch (...) { | |
| PyObject* radd = PyObject_GetAttrString(other_obj, "__radd__"); | |
| if (radd) { | |
| bool has_callable_radd = PyCallable_Check(radd); | |
| Py_DECREF(radd); | |
| if (has_callable_radd) { | |
| Py_INCREF(Py_NotImplemented); | |
| return Py_NotImplemented; | |
| } | |
| } | |
| throw; | |
| } | |
| value = HandleReflectedMethodFallback(other_obj, "__radd__", [&]() { | |
| return CastPyArg2Scalar(other_obj, "__add__", 0); | |
| }); |
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@ ## develop #73833 +/- ## ========================================== Coverage ? 0 ========================================== Files ? 0 Lines ? 0 Branches ? 0 ========================================== Hits ? 0 Misses ? 0 Partials ? 0 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
3 participants
PR Category
User Experience
PR Types
Bug fixes
Description
Pcard-75624
__rxx__方法，将原来抛出的所有异常都改为了 NotImplement，导致正常的异常信息无法正确显示，在 Python 侧会全部显示NotImplement。因此将触发__rxx__的条件放到了相关运算的 pybind 代码中，只有当所有逻辑失败时，才会运行到 catch分支，触发__rxx__。这样不会影响原来的正常异常抛出；单测添加了不同异常触发的判断测试，确保不影响正常的其它异常信息捕获__rxx__系列运算触发机制cc @BeingGod