-
Notifications
You must be signed in to change notification settings - Fork 275
API: use __array_ufunc__ as numpy ufunc enter
#1163
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
Replaces custom rich comparison methods for MatrixExpr and MatrixExprCons with __array_ufunc__ implementations, leveraging numpy's ufuncs for elementwise comparisons. Removes legacy helper functions and streamlines operator handling, improving maintainability and performance.
Deleted the 'include "matrix.pxi"' line from expr.pxi as it is no longer needed. This helps clean up the code and avoid unnecessary dependencies.
Updated type checks from MatrixExpr to np.ndarray throughout expr.pxi to improve compatibility with NumPy arrays and simplify matrix expression handling.
Corrects the condition to check for NotImplemented in MatrixExpr's __array_ufunc__ method, ensuring proper delegation to the superclass implementation.
Update the test to assert that the result of 1D @ 1D matrix multiplication is of type Expr instead of MatrixExpr.
MatrixExprCons now only supports the '<=' and '>=' ufuncs, raising NotImplementedError for all others. This clarifies and enforces the intended usage of the class.
Refactored test_matrix_sum_result to use a view of MatrixExpr for sum operation, aligning the test with the expected usage pattern and simplifying result comparison.
Simplifies the __array_ufunc__ method by directly returning a MatrixExpr view when the result is a numpy ndarray, improving code clarity.
Updated type checks in exp, log, sqrt, sin, and cos functions to use MatrixExpr instead of np.ndarray. This change ensures that matrix-specific expression handling is applied only to MatrixExpr instances.
Eliminated explicit method declarations from the MatrixExpr and MatrixExprCons classes in the type stub, leaving only ellipses. This simplifies the type hints and may reflect a change in how these classes are intended to be used or documented.
Added entry noting that MatrixExpr and MatrixExprCons now use the `__array_ufunc__` protocol to control all numpy.ufunc inputs and outputs.
|
Waiting for #1157. And |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR refactors the MatrixExpr and MatrixExprCons classes to use NumPy's __array_ufunc__ protocol instead of manually overriding individual operators. This provides comprehensive control over all numpy universal function operations on these matrix types.
Changes:
- Replaced manual operator overrides (
__add__,__mul__,__le__,__ge__, etc.) with__array_ufunc__implementations in bothMatrixExprandMatrixExprConsclasses - Added vectorized comparison functions (
_vec_le,_vec_ge,_vec_eq) usingoperatormodule - Simplified type stubs by removing explicit method signatures since
__array_ufunc__handles operations automatically
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/pyscipopt/matrix.pxi | Replaces manual operator overrides with __array_ufunc__ protocol implementation for comprehensive numpy ufunc control; adds vectorized comparison helpers |
| src/pyscipopt/scip.pyi | Simplifies type stubs by removing explicit operator method signatures (now handled by __array_ufunc__) |
| CHANGELOG.md | Documents the adoption of __array_ufunc__ protocol for numpy ufunc control |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Added a cimport for Expr from pyscipopt.scip to enable usage of the Expr type in this module.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Replaces use of argument unpacking (*args) with explicit indexing (args[0], args[1]) in calls to _vec_le, _vec_ge, and _vec_eq within MatrixExpr and MatrixExprCons. This change clarifies argument passing and may prevent potential issues with argument order or count.
Relocated the definition of _vec_evaluate to group all np.frompyfunc vectorized operator functions together for better code organization and readability.
fixes #1117. Some other methods will return
np.ndarraytype fromMatrixExpr. And__array_ufunc__could fix them all.and closes #1139