-
Notifications
You must be signed in to change notification settings - Fork 275
Feature: Expr and GenExpr support numpy unary func like np.sin
#1170
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
base: master
Are you sure you want to change the base?
Conversation
Introduced the ExprLike base class with __array_ufunc__ to enable numpy universal function (ufunc) support for Expr and GenExpr. Replaced standalone exp, log, sqrt, sin, and cos functions with numpy equivalents and mapped them for ufunc dispatch. This change improves interoperability with numpy and simplifies the codebase.
Added a new ExprLike base class and made Expr inherit from it. This refactoring prepares the codebase for future extensions or polymorphism involving expression-like objects.
Corrects the method call to use the first argument in the unary ufunc mapping, ensuring the correct object is used when applying numpy universal functions.
Introduces tests for unary operations such as abs, sin, and sqrt, including their numpy equivalents, to ensure correct string representations and compatibility with numpy functions.
| return expr | ||
|
|
||
|
|
||
| cdef class ExprLike: |
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.
ExprLike is the base class and also a duck type.
It defines the behavior, and its subclass defines the data.
I will use ExprLike to split Variable and Expr in the future.
Added a missing colon to the ExprLike class definition in scip.pxd to conform with Python/Cython syntax.
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 adds support for NumPy unary functions (np.sin, np.cos, np.sqrt, np.exp, np.log, np.absolute) to work with Expr and GenExpr objects by implementing the __array_ufunc__ protocol.
Changes:
- Introduces a new
ExprLikebase class that implements__array_ufunc__and unary operation methods - Makes
ExprandGenExprinherit fromExprLiketo support NumPy ufuncs - Replaces custom function implementations with NumPy function aliases
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 7 comments.
| File | Description |
|---|---|
| src/pyscipopt/scip.pxd | Adds ExprLike base class declaration and updates Expr inheritance hierarchy |
| src/pyscipopt/expr.pxi | Implements ExprLike with __array_ufunc__ support, consolidates unary methods, aliases functions to NumPy equivalents |
| tests/test_expr.py | Adds test coverage for both custom functions and NumPy ufuncs with single expressions and arrays |
| CHANGELOG.md | Documents the new NumPy unary function support feature |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Updated test assertions in test_unary to expect variable names 'x', 'y', and 'z' instead of 'x1'. This aligns the tests with the current string representation of expressions.
Updated the expected string in test_unary to match the correct output format by removing commas between list elements.
Corrected the expected string in test_unary to use 'sin' instead of 'abs' for the sin([x, y, z]) test case.
Introduces a cpdef _evaluate method to the Constant class, returning the constant's value. This provides a consistent evaluation interface for expressions.
Removes the NotImplementedError expectation when raising genexpr to sqrt(2) and instead asserts the result is a GenExpr. Adds a new test to expect TypeError when raising to sqrt('2').
The test_unary function was moved and modified to test unary operations on lists containing two variables (x, y) instead of three (x, y, z). This streamlines the tests and aligns them with the current requirements.
Introduced a new ExprLike base class to encapsulate common mathematical methods such as __abs__, exp, log, sqrt, sin, and cos. Updated Expr and GenExpr to inherit from ExprLike, reducing code duplication and improving maintainability.
| assert isinstance(1/x + genexpr, GenExpr) | ||
| assert isinstance(1/x**1.5 - genexpr, GenExpr) | ||
| assert isinstance(y/x - exp(genexpr), GenExpr) | ||
| # sqrt(2) is not a constant expression 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.
pyscipopt.sqrt(2) = np.sqrt(2). It's a constant now, not a Constant(GenExpr)
Introduces the __array_ufunc__ method to the ExprLike class in the type stubs, enabling better compatibility with NumPy ufuncs.
…nto expr/unary
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #1170 +/- ##
==========================================
- Coverage 55.87% 55.50% -0.37%
==========================================
Files 25 25
Lines 5502 5495 -7
==========================================
- Hits 3074 3050 -24
- Misses 2428 2445 +17 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Changed 'import numpy' to 'import numpy as np' and updated type annotations to use 'np.ndarray' instead of 'numpy.ndarray' in the scip.pyi stub file. Also added UNARY_MAPPER as a Dict[np.ufunc, str].
Replaces 'Incomplete' type hints with more specific types (np.ufunc and str) for the __array_ufunc__ methods in ExprLike, MatrixExpr, and MatrixExprCons classes to improve type accuracy.
Reordered the declarations of math functions and added missing entries for log, sin, and sqrt in the scip.pyi stub file to improve completeness and maintain consistency.
Moved the Operator type annotation below PATCH to maintain consistent ordering of variable declarations in the scip.pyi file.
The @disjoint_base decorator was removed from the ExprLike class in the type stub. This may reflect a change in the class hierarchy or decorator usage.
| log: Incomplete | ||
| sin: Incomplete | ||
| cos: Incomplete | ||
| sqrt: Incomplete |
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.
put them together
| def cos(expr): | ||
| """returns expression with cos-function""" | ||
| if isinstance(expr, MatrixExpr): | ||
| unary_exprs = np.empty(shape=expr.shape, dtype=object) | ||
| for idx in np.ndindex(expr.shape): | ||
| unary_exprs[idx] = UnaryExpr(Operator.cos, buildGenExprObj(expr[idx])) | ||
| return unary_exprs.view(MatrixGenExpr) | ||
| else: | ||
| return UnaryExpr(Operator.cos, buildGenExprObj(expr)) |
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.
- Before:
- number:
cos(2)will return aUnaryExprto wrap2. np.ndarray(..., dtype=np.number): can't handle this.
- number:
- Now:
- number:
cos(2)will return constantcos(2)=-0.416. Both Expr and GenExpr can handle numbers. np.ndarray(..., dtype=np.number): returnnp.ndarray(..., dtype=np.number).
- number:
Replaces numpy function references in UNARY_MAPPER with local aliases (exp, log, sqrt, sin, cos) to ensure correct mapping and avoid issues with numpy function identity.
Replaces the use of Dict from typing with the built-in dict for the UNARY_MAPPER type annotation. This modernizes the type hint and removes an unused import.
Deleted the UNARY_MAPPER dictionary from scip.pyi as it is no longer needed or used in the type stub.
Expr and GenExpr support numpy unary functions like
np.sin.Now we can do this
Before this, we couldn't