-
Notifications
You must be signed in to change notification settings - Fork 280
Speed up -SumExpr, -ProdExpr and -Constant
#1179
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
28d3adc
1be74c8
e4351fa
fb9fcc8
f55c222
cb349ce
5896012
6387cfa
eac8db9
fecba06
02e32b5
bd280f6
2e97cc7
67ce45d
40945ad
ef034c4
86678e2
394c682
d348d48
b881b12
39c036c
3d2bff0
1c6598f
2b0cc32
70ef34e
05dd131
af2f83a
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 |
|---|---|---|
|
|
@@ -347,8 +347,8 @@ cdef class Expr: | |
| else: | ||
| raise TypeError(f"Unsupported base type {type(other)} for exponentiation.") | ||
|
|
||
| def __neg__(self): | ||
| return Expr({v:-c for v,c in self.terms.items()}) | ||
| def __neg__(self) -> Expr: | ||
| return -1.0 * self | ||
|
|
||
| def __sub__(self, other): | ||
| return self + (-other) | ||
|
|
@@ -712,14 +712,31 @@ cdef class SumExpr(GenExpr): | |
| self.coefs = [] | ||
| self.children = [] | ||
| self._op = Operator.add | ||
|
|
||
| def __neg__(self) -> SumExpr: | ||
| cdef int i = 0, n = len(self.coefs) | ||
| cdef list coefs = [0.0] * n | ||
| cdef double[:] dest_view = coefs | ||
| cdef double[:] src_view = self.coefs | ||
|
|
||
| for i in range(n): | ||
| dest_view[i] = -src_view[i] | ||
|
|
||
| cdef SumExpr res = SumExpr.__new__(SumExpr) | ||
| res.coefs = coefs | ||
| res.children = self.children.copy() | ||
| res.constant = -self.constant | ||
| res._op = Operator.add | ||
| return res | ||
|
|
||
| def __repr__(self): | ||
| return self._op + "(" + str(self.constant) + "," + ",".join(map(lambda child : child.__repr__(), self.children)) + ")" | ||
|
|
||
| cpdef double _evaluate(self, Solution sol) except *: | ||
| cdef double res = self.constant | ||
| cdef int i = 0, n = len(self.children) | ||
| cdef list children = self.children | ||
| cdef list coefs = self.coefs | ||
| cdef double[:] coefs = self.coefs | ||
| for i in range(n): | ||
| res += <double>coefs[i] * (<GenExpr>children[i])._evaluate(sol) | ||
|
Comment on lines
737
to
741
|
||
| return res | ||
|
|
@@ -735,6 +752,13 @@ cdef class ProdExpr(GenExpr): | |
| self.children = [] | ||
| self._op = Operator.prod | ||
|
|
||
| def __neg__(self) -> ProdExpr: | ||
| cdef ProdExpr res = ProdExpr.__new__(ProdExpr) | ||
| res.constant = -self.constant | ||
| res.children = self.children.copy() | ||
| res._op = Operator.prod | ||
| return res | ||
|
|
||
| def __repr__(self): | ||
| return self._op + "(" + str(self.constant) + "," + ",".join(map(lambda child : child.__repr__(), self.children)) + ")" | ||
|
|
||
|
|
@@ -804,11 +828,16 @@ cdef class UnaryExpr(GenExpr): | |
|
|
||
| # class for constant expressions | ||
| cdef class Constant(GenExpr): | ||
|
|
||
| cdef public number | ||
|
|
||
| def __init__(self,number): | ||
| self.number = number | ||
| self._op = Operator.const | ||
|
|
||
| def __neg__(self) -> Constant: | ||
| return Constant(-self.number) | ||
|
|
||
| def __repr__(self): | ||
| return str(self.number) | ||
|
|
||
|
|
||
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.
coefsandself.coefsare Python lists (see__init__and theappend/extendusage), so assigning them todouble[:]memoryviews (dest_view/src_view) will fail at runtime because lists don't provide the buffer interface. To keep this optimization safe, either (a) avoid typed memoryviews here and negate with plain list operations, or (b) switchSumExpr.coefsto a buffer-compatible container (e.g.,array('d')) consistently across the class (includingcopy()and all mutating sites).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.
Should fix #1179 (comment) first.