Skip to content
Open
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
### Changed
- Speed up `constant * Expr` via C-level API
- Speed up `Term.__eq__` via the C-level API
- Speed up `Expr.__add__` and `Expr.__iadd__` via the C-level API
### Removed
- Removed outdated warning about Make build system incompatibility
- Removed `Term.ptrtuple` to optimize `Term` memory usage
Expand Down
67 changes: 40 additions & 27 deletions src/pyscipopt/expr.pxi
Original file line number Diff line number Diff line change
Expand Up @@ -302,42 +302,33 @@ cdef class Expr(ExprLike):
return iter(self.terms)

def __add__(self, other):
left = self
right = other
terms = left.terms.copy()

if isinstance(right, Expr):
# merge the terms by component-wise addition
for v,c in right.terms.items():
terms[v] = terms.get(v, 0.0) + c
elif _is_number(right):
c = float(right)
terms[CONST] = terms.get(CONST, 0.0) + c
elif isinstance(right, GenExpr):
return buildGenExprObj(left) + right
elif isinstance(right, np.ndarray):
return right + left
if _is_number(other):
terms = self.terms.copy()
terms[CONST] = terms.get(CONST, 0.0) + <double>other
return Expr(terms)
elif isinstance(other, Expr):
return Expr(_to_dict(self, other, copy=True))
elif isinstance(other, GenExpr):
Comment on lines 304 to +311
Copy link

Copilot AI Apr 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

__add__ now checks _is_number(other) before isinstance(other, Expr). Since _is_number calls float(e) and relies on exceptions for non-numeric types, adding two Expr objects will raise/catch TypeError on every call, which is avoidable overhead in the hot path. Consider checking Expr/GenExpr/np.ndarray first (or replacing _is_number with a cheaper non-exception-based numeric check) and only then falling back to number handling.

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

_is_number will be optimized in #1179

return buildGenExprObj(self) + other
elif isinstance(other, np.ndarray):
return other + self
else:
raise TypeError(f"Unsupported type {type(right)}")

return Expr(terms)
raise TypeError(f"unsupported type {type(other).__name__!r}")

def __iadd__(self, other):
if isinstance(other, Expr):
for v,c in other.terms.items():
self.terms[v] = self.terms.get(v, 0.0) + c
elif _is_number(other):
c = float(other)
self.terms[CONST] = self.terms.get(CONST, 0.0) + c
if _is_number(other):
self.terms[CONST] = self.terms.get(CONST, 0.0) + <double>other
return self
elif isinstance(other, Expr):
_to_dict(self, other, copy=False)
return self
Comment thread
Zeroto521 marked this conversation as resolved.
elif isinstance(other, GenExpr):
# is no longer in place, might affect performance?
# can't do `self = buildGenExprObj(self) + other` since I get
# TypeError: Cannot convert pyscipopt.scip.SumExpr to pyscipopt.scip.Expr
return buildGenExprObj(self) + other
else:
raise TypeError(f"Unsupported type {type(other)}")

return self
raise TypeError(f"unsupported type {type(other).__name__!r}")

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

cdef dict _to_dict(Expr expr, Expr other, bool copy = True):
cdef dict children = expr.terms.copy() if copy else expr.terms
cdef Py_ssize_t pos = <Py_ssize_t>0
cdef PyObject* k_ptr = NULL
cdef PyObject* v_ptr = NULL
cdef PyObject* old_v_ptr = NULL
cdef double other_v
cdef object k_obj

while PyDict_Next(other.terms, &pos, &k_ptr, &v_ptr):
if (other_v := <double>(<object>v_ptr)) == 0:
continue

k_obj = <object>k_ptr
old_v_ptr = PyDict_GetItem(children, k_obj)
if old_v_ptr != NULL:
children[k_obj] = <double>(<object>old_v_ptr) + other_v
else:
children[k_obj] = <object>v_ptr
Comment thread
Zeroto521 marked this conversation as resolved.
Outdated

return children


def expr_to_nodes(expr):
'''transforms tree to an array of nodes. each node is an operator and the position of the
Expand Down
25 changes: 25 additions & 0 deletions tests/test_expr.py
Original file line number Diff line number Diff line change
Expand Up @@ -328,3 +328,28 @@ def test_term_eq():
assert t3 != t4 # same length, but different term
assert t1 != t3 # different length
assert t1 != "not a term" # different type


def test_Expr_add_Expr():
m = Model()
x = m.addVar(name="x")
y = m.addVar(name="y")

e1 = -x + 1
e2 = y - 1
e3 = e1 + e2
assert str(e1) == "Expr({Term(x): -1.0, Term(): 1.0})"
assert str(e2) == "Expr({Term(y): 1.0, Term(): -1.0})"
assert str(e3) == "Expr({Term(x): -1.0, Term(): 0.0, Term(y): 1.0})"


def test_Expr_iadd_Expr():
m = Model()
x = m.addVar(name="x")
y = m.addVar(name="y")

e1 = -x + 1
e2 = y - 1
e1 += e2
assert str(e1) == "Expr({Term(x): -1.0, Term(): 0.0, Term(y): 1.0})"
assert str(e2) == "Expr({Term(y): 1.0, Term(): -1.0})"
Loading