Speed up Constant @ MatrixExpr
#1159
Conversation
Added numpy to build requirements in pyproject.toml and setup.py, ensuring numpy headers are included during compilation. Refactored matrix.pxi to improve matrix operation support, including custom __array_ufunc__ handling for dot/matmul, utility functions for type checking and array conversion, and a vectorized _core_dot implementation for efficient matrix multiplication.
Introduces a new parameterized test for matrix dot product performance and updates an assertion in test_matrix_matmul_return_type to expect Expr instead of MatrixExpr for 1D @ 1D operations.
Updated type checks throughout expr.pxi to use np.ndarray instead of MatrixExpr, improving compatibility with numpy arrays. Also adjusted matrix.pxi to ensure ufunc results are returned as MatrixExpr views when appropriate.
Deleted unnecessary conversion of the 'out' keyword argument to an array in MatrixExpr.__array_ufunc__, as it is not required for correct operation.
Deleted the overridden __matmul__ method in MatrixExpr, reverting to the default numpy ndarray behavior for matrix multiplication. This simplifies the class and avoids unnecessary type casting.
Joao-Dionisio
left a comment
Joao-Dionisio
left a comment
There was a problem hiding this comment.
There are some things I need clarification on.
Also, changes should be, as much as possible, minimal, in the sense that they should only fix the one issue in the PR, otherwise it gets hard to follow. Not saying you're not doing this, just that there is a bunch of code I don't understand :)
Introduces a new _core_dot function to handle matrix multiplication between constant arrays and arrays of Expr objects, supporting both 1-D and N-D cases. The original _core_dot is renamed to _core_dot_2d and improved for clarity and efficiency. Updates __array_ufunc__ to use the new logic, ensuring correct handling of mixed-type matrix operations.
Adjusted tests in test_matrix_matmul_return_type to check the return type when performing matrix multiplication with numpy arrays and matrix variables, including new cases for ND arrays. Ensures correct type inference for various matrix multiplication scenarios.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #1159 +/- ##
==========================================
+ Coverage 55.07% 55.28% +0.21%
==========================================
Files 24 24
Lines 5420 5484 +64
==========================================
+ Hits 2985 3032 +47
- Misses 2435 2452 +17 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Replaces the _is_num_dt helper function with direct dtype.kind checks for numeric types in _core_dot. This simplifies the code and removes an unnecessary inline function.
|
I'm sure you're noticed by now @Zeroto521 , but I'm not a user of numpy :) So, to be safe: are the end results of your changes something that a non-expert user would be expecting, syntax-wise? While performance is amazing, the goal is also to be as attrition-free for the user as possible. |
Added detailed type annotations and a docstring to the MatrixExpr.__array_ufunc__ method for improved clarity and maintainability. Also clarified handling of ufunc methods and argument unboxing.
|
Hey @Zeroto521 , please take a moment to address this comment. Is the end result what would be expected in the numpy world, and in gurobipy/xpresspy? |
Renamed the test function to better reflect its purpose of testing performance for matrix dot operations.
Introduces test_matrix_dot_value to verify correct value retrieval from matrix variable dot products using getVal. Ensures expected results for both 1D and higher-dimensional dot operations.
|
2 participants
closes to #1156