-
Notifications
You must be signed in to change notification settings - Fork 284
Speed up sol[UnaryExpr] via C-level math and refactor UnaryExpr
#1224
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?
Changes from all commits
0988f86
3c7a981
166dc21
647f8ac
fadf4e1
c27fe01
e5bdabc
8294d8a
94131e9
c681760
c0750cb
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -42,7 +42,6 @@ | |
| # which should, in princple, modify the expr. However, since we do not implement __isub__, __sub__ | ||
| # gets called (I guess) and so a copy is returned. | ||
| # Modifying the expression directly would be a bug, given that the expression might be re-used by the user. </pre> | ||
| import math | ||
| from typing import TYPE_CHECKING, Literal, Union | ||
|
|
||
| import numpy as np | ||
|
|
@@ -54,6 +53,12 @@ from cpython.number cimport PyNumber_Check | |
| from cpython.object cimport Py_LE, Py_EQ, Py_GE, Py_TYPE | ||
| from cpython.ref cimport PyObject | ||
| from cpython.tuple cimport PyTuple_GET_ITEM | ||
| from libc.math cimport cos as c_cos | ||
| from libc.math cimport exp as c_exp | ||
| from libc.math cimport fabs as c_fabs | ||
| from libc.math cimport log as c_log | ||
| from libc.math cimport sqrt as c_sqrt | ||
| from libc.math cimport sin as c_sin | ||
|
|
||
| cimport numpy as cnp | ||
| from pyscipopt.scip cimport Variable, Solution | ||
|
|
@@ -281,23 +286,28 @@ cdef class ExprLike: | |
| def __pos__(self, /) -> Union[Expr, GenExpr]: | ||
| return self.copy() | ||
|
|
||
| def __abs__(self) -> GenExpr: | ||
| return UnaryExpr(Operator.fabs, buildGenExprObj(self)) | ||
| def __abs__(self, /) -> AbsExpr: | ||
| return AbsExpr(Operator.fabs, buildGenExprObj(self)) | ||
|
|
||
| def exp(self) -> GenExpr: | ||
| return UnaryExpr(Operator.exp, buildGenExprObj(self)) | ||
| def exp(self, /) -> ExpExpr: | ||
| return ExpExpr(Operator.exp, buildGenExprObj(self)) | ||
|
|
||
| def log(self) -> GenExpr: | ||
| return UnaryExpr(Operator.log, buildGenExprObj(self)) | ||
| def log(self, /) -> LogExpr: | ||
| return LogExpr(Operator.log, buildGenExprObj(self)) | ||
|
|
||
| def sqrt(self) -> GenExpr: | ||
| return UnaryExpr(Operator.sqrt, buildGenExprObj(self)) | ||
| def sqrt(self, /) -> SqrtExpr: | ||
| return SqrtExpr(Operator.sqrt, buildGenExprObj(self)) | ||
|
|
||
| def sin(self) -> GenExpr: | ||
| return UnaryExpr(Operator.sin, buildGenExprObj(self)) | ||
| def sin(self, /) -> SinExpr: | ||
| return SinExpr(Operator.sin, buildGenExprObj(self)) | ||
|
|
||
| def cos(self) -> GenExpr: | ||
| return UnaryExpr(Operator.cos, buildGenExprObj(self)) | ||
| def cos(self, /) -> CosExpr: | ||
| return CosExpr(Operator.cos, buildGenExprObj(self)) | ||
|
|
||
| cpdef double _evaluate(self, Solution sol) except *: | ||
| raise NotImplementedError( | ||
| f"{self.__class__.__name__!s} need to implement _evaluate() method" | ||
| ) | ||
|
|
||
| cdef ExprLike copy(self, bint copy=True): | ||
| raise NotImplementedError( | ||
|
|
@@ -828,24 +838,54 @@ cdef class PowExpr(GenExpr): | |
| return res | ||
|
|
||
|
|
||
| # Exp, Log, Sqrt, Sin, Cos Expressions | ||
| cdef class UnaryExpr(GenExpr): | ||
|
|
||
| def __init__(self, op, expr): | ||
| self.children = [] | ||
| self.children.append(expr) | ||
| self._op = op | ||
|
|
||
| def __abs__(self) -> UnaryExpr: | ||
| if self._op == "abs": | ||
| return <UnaryExpr>self.copy() | ||
| return UnaryExpr(Operator.fabs, self) | ||
|
|
||
| def __repr__(self): | ||
| def __repr__(self) -> str: | ||
| return self._op + "(" + self.children[0].__repr__() + ")" | ||
|
|
||
|
|
||
| cdef class AbsExpr(UnaryExpr): | ||
|
|
||
| def __abs__(self) -> AbsExpr: | ||
| return <AbsExpr>self.copy() | ||
|
|
||
| cpdef double _evaluate(self, Solution sol) except *: | ||
| return c_fabs((<GenExpr>self.children[0])._evaluate(sol)) | ||
|
|
||
|
Zeroto521 marked this conversation as resolved.
|
||
|
|
||
| cdef class ExpExpr(UnaryExpr): | ||
|
|
||
| cpdef double _evaluate(self, Solution sol) except *: | ||
| return c_exp((<GenExpr>self.children[0])._evaluate(sol)) | ||
|
|
||
|
|
||
| cdef class LogExpr(UnaryExpr): | ||
|
|
||
| cpdef double _evaluate(self, Solution sol) except *: | ||
| return c_log((<GenExpr>self.children[0])._evaluate(sol)) | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Now, things like
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should we bring back these Python features?
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm not sure what the best option is, it goes a bit too much into the software engineering side of things. I'm leaning towards keeping the old behavior, but maybe @mmghannam can give a better opinion. |
||
|
|
||
|
|
||
| cdef class SqrtExpr(UnaryExpr): | ||
|
|
||
| cpdef double _evaluate(self, Solution sol) except *: | ||
| return c_sqrt((<GenExpr>self.children[0])._evaluate(sol)) | ||
|
|
||
|
|
||
| cdef class SinExpr(UnaryExpr): | ||
|
|
||
| cpdef double _evaluate(self, Solution sol) except *: | ||
| return c_sin((<GenExpr>self.children[0])._evaluate(sol)) | ||
|
|
||
|
|
||
| cdef class CosExpr(UnaryExpr): | ||
|
|
||
| cpdef double _evaluate(self, Solution sol) except *: | ||
| cdef double res = (<GenExpr>self.children[0])._evaluate(sol) | ||
| return math.fabs(res) if self._op == "abs" else getattr(math, self._op)(res) | ||
| return c_cos((<GenExpr>self.children[0])._evaluate(sol)) | ||
|
|
||
|
|
||
| # class for constant expressions | ||
|
|
||
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.__abs__can replaceUnaryExpr(Operator.fabs, self)