Skip to content

Commit 33ff48d

Browse files
committed
Optimize Expr __add__ and __iadd__
Refactor Expr.__add__ and __iadd__ to centralize term merging in a new C helper _to_dict. _to_dict iterates other.terms with PyDict_Next, skips zero coefficients, and either copies or mutates the target dict depending on the copy flag. __add__ now handles numeric additions first and uses _to_dict(copy=True) for Expr+Expr; __iadd__ uses _to_dict(copy=False) to perform in-place merges and returns self. GenExpr and numpy-array cases are preserved. Error messages were slightly adjusted and numeric values are cast to double for correctness and performance.
1 parent d9e9530 commit 33ff48d

1 file changed

Lines changed: 40 additions & 27 deletions

File tree

src/pyscipopt/expr.pxi

Lines changed: 40 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -302,42 +302,33 @@ cdef class Expr(ExprLike):
302302
return iter(self.terms)
303303

304304
def __add__(self, other):
305-
left = self
306-
right = other
307-
terms = left.terms.copy()
308-
309-
if isinstance(right, Expr):
310-
# merge the terms by component-wise addition
311-
for v,c in right.terms.items():
312-
terms[v] = terms.get(v, 0.0) + c
313-
elif _is_number(right):
314-
c = float(right)
315-
terms[CONST] = terms.get(CONST, 0.0) + c
316-
elif isinstance(right, GenExpr):
317-
return buildGenExprObj(left) + right
318-
elif isinstance(right, np.ndarray):
319-
return right + left
305+
if _is_number(other):
306+
terms = self.terms.copy()
307+
terms[CONST] = terms.get(CONST, 0.0) + <double>other
308+
return Expr(terms)
309+
elif isinstance(other, Expr):
310+
return Expr(_to_dict(self, other, copy=True))
311+
elif isinstance(other, GenExpr):
312+
return buildGenExprObj(self) + other
313+
elif isinstance(other, np.ndarray):
314+
return other + self
320315
else:
321-
raise TypeError(f"Unsupported type {type(right)}")
322-
323-
return Expr(terms)
316+
raise TypeError(f"unsupported type {type(other).__name__!r}")
324317

325318
def __iadd__(self, other):
326-
if isinstance(other, Expr):
327-
for v,c in other.terms.items():
328-
self.terms[v] = self.terms.get(v, 0.0) + c
329-
elif _is_number(other):
330-
c = float(other)
331-
self.terms[CONST] = self.terms.get(CONST, 0.0) + c
319+
if _is_number(other):
320+
self.terms[CONST] = self.terms.get(CONST, 0.0) + <double>other
321+
return self
322+
elif isinstance(other, Expr):
323+
_to_dict(self, other, copy=False)
324+
return self
332325
elif isinstance(other, GenExpr):
333326
# is no longer in place, might affect performance?
334327
# can't do `self = buildGenExprObj(self) + other` since I get
335328
# TypeError: Cannot convert pyscipopt.scip.SumExpr to pyscipopt.scip.Expr
336329
return buildGenExprObj(self) + other
337330
else:
338-
raise TypeError(f"Unsupported type {type(other)}")
339-
340-
return self
331+
raise TypeError(f"unsupported type {type(other).__name__!r}")
341332

342333
def __mul__(self, other):
343334
if isinstance(other, np.ndarray):
@@ -1031,6 +1022,28 @@ cdef inline object _wrap_ufunc(object x, object ufunc):
10311022
return res.view(MatrixGenExpr) if isinstance(res, np.ndarray) else res
10321023
return ufunc(_to_const(x))
10331024

1025+
cdef dict _to_dict(Expr expr, Expr other, bool copy = True):
1026+
cdef dict children = expr.terms.copy() if copy else expr.terms
1027+
cdef Py_ssize_t pos = <Py_ssize_t>0
1028+
cdef PyObject* k_ptr = NULL
1029+
cdef PyObject* v_ptr = NULL
1030+
cdef PyObject* old_v_ptr = NULL
1031+
cdef double other_v
1032+
cdef object k_obj
1033+
1034+
while PyDict_Next(other.terms, &pos, &k_ptr, &v_ptr):
1035+
if (other_v := <double>(<object>v_ptr)) == 0:
1036+
continue
1037+
1038+
k_obj = <object>k_ptr
1039+
old_v_ptr = PyDict_GetItem(children, k_obj)
1040+
if old_v_ptr != NULL:
1041+
children[k_obj] = <double>(<object>old_v_ptr) + other_v
1042+
else:
1043+
children[k_obj] = <object>v_ptr
1044+
1045+
return children
1046+
10341047

10351048
def expr_to_nodes(expr):
10361049
'''transforms tree to an array of nodes. each node is an operator and the position of the

0 commit comments

Comments
 (0)