diff --git a/CHANGELOG.md b/CHANGELOG.md index 29b8597bf..5ff26d702 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,6 +17,7 @@ - Made expression fields read-only to prevent accidental runtime modification - Methods taking a list of variables (e.g. `addConsKnapsack()`) no longer convert it to a NumPy array to check its dimension - Speed up `PowExpr._evaluate` via the C-level API +- Speed up `Term.__init__`, `Term.__eq__`, and `Term.__mul__` via the C-level API - `PowExpr._evaluate` now raises `ValueError("math domain error")` for a negative base with a non-integer exponent and `OverflowError` when the result overflows. ### Removed diff --git a/src/pyscipopt/expr.pxi b/src/pyscipopt/expr.pxi index 46414b2ca..ce89f558e 100644 --- a/src/pyscipopt/expr.pxi +++ b/src/pyscipopt/expr.pxi @@ -53,7 +53,7 @@ from cpython.long cimport PyLong_Check 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 cpython.tuple cimport PyTuple_GET_ITEM, PyTuple_GET_SIZE from libc.math cimport cos as c_cos from libc.math cimport exp as c_exp from libc.math cimport fabs as c_fabs @@ -79,8 +79,25 @@ cdef class Term: cdef Py_ssize_t hashval def __init__(self, *vartuple: Variable): - self.vartuple = tuple(sorted(vartuple, key=lambda v: v.getIndex())) - self.hashval = hash(tuple(v.ptr() for v in self.vartuple)) + cdef Py_ssize_t n = PyTuple_GET_SIZE(vartuple) + cdef Variable var + + if n == 0: + # The constant term; the empty args tuple is already canonical. + self.vartuple = vartuple + self.hashval = hash(vartuple) + + elif n == 1: + # A single variable is trivially sorted, so avoid sorted and the generator. + # `vartuple` is the exact tuple built from *args, so PyTuple_GET_ITEM needs + # no bounds check; the `` cast keeps the Variable type check. + var = PyTuple_GET_ITEM(vartuple, 0) + self.vartuple = vartuple + self.hashval = hash((var.ptr(),)) + + else: + self.vartuple = tuple(sorted(vartuple, key=lambda v: v.getIndex())) + self.hashval = hash(tuple(v.ptr() for v in self.vartuple)) def __getitem__(self, idx): return self.vartuple[idx] @@ -94,12 +111,12 @@ cdef class Term: if Py_TYPE(other) is not Term: return False - cdef int n = len(self) + cdef Py_ssize_t n = PyTuple_GET_SIZE(self.vartuple) cdef Term _other = other - if n != len(_other) or self.hashval != _other.hashval: + if n != PyTuple_GET_SIZE(_other.vartuple) or self.hashval != _other.hashval: return False - cdef int i + cdef Py_ssize_t i cdef Variable var1, var2 for i in range(n): var1 = PyTuple_GET_ITEM(self.vartuple, i) @@ -109,41 +126,52 @@ cdef class Term: return True def __len__(self): - return len(self.vartuple) + return PyTuple_GET_SIZE(self.vartuple) def __mul__(self, Term other): # NOTE: This merge algorithm requires a sorted `Term.vartuple`. # This should be ensured in the constructor of Term. - cdef int n1 = len(self) - cdef int n2 = len(other) + cdef Py_ssize_t n1 = PyTuple_GET_SIZE(self.vartuple) + cdef Py_ssize_t n2 = PyTuple_GET_SIZE(other.vartuple) if n1 == 0: return other if n2 == 0: return self - cdef list vartuple = [None] * (n1 + n2) - cdef int i = 0, j = 0, k = 0 + cdef Py_ssize_t m = n1 + n2 + cdef list vartuple = [None] * m + # The variable pointers are collected during the merge, so the hash can be + # built without a generator that calls the Python-level `Variable.ptr()` + # once per variable. + cdef list ptrs = [None] * m + cdef Py_ssize_t i = 0, j = 0, k = 0 cdef Variable var1, var2 while i < n1 and j < n2: var1 = PyTuple_GET_ITEM(self.vartuple, i) var2 = PyTuple_GET_ITEM(other.vartuple, j) if var1.getIndex() <= var2.getIndex(): vartuple[k] = var1 + ptrs[k] = var1.ptr() i += 1 else: vartuple[k] = var2 + ptrs[k] = var2.ptr() j += 1 k += 1 while i < n1: - vartuple[k] = PyTuple_GET_ITEM(self.vartuple, i) + var1 = PyTuple_GET_ITEM(self.vartuple, i) + vartuple[k] = var1 + ptrs[k] = var1.ptr() i += 1 k += 1 while j < n2: - vartuple[k] = PyTuple_GET_ITEM(other.vartuple, j) + var2 = PyTuple_GET_ITEM(other.vartuple, j) + vartuple[k] = var2 + ptrs[k] = var2.ptr() j += 1 k += 1 cdef Term res = Term.__new__(Term) res.vartuple = tuple(vartuple) - res.hashval = hash(tuple(v.ptr() for v in res.vartuple)) + res.hashval = hash(tuple(ptrs)) return res def __repr__(self) -> str: @@ -153,11 +181,11 @@ cdef class Term: cdef double res = 1.0 cdef SCIP* scip_ptr = sol.scip cdef SCIP_SOL* sol_ptr = sol.sol - cdef int i = 0, n = len(self) + cdef Py_ssize_t i, n = PyTuple_GET_SIZE(self.vartuple) cdef Variable var for i in range(n): - var = self.vartuple[i] + var = PyTuple_GET_ITEM(self.vartuple, i) res *= SCIPgetSolVal(scip_ptr, sol_ptr, var.scip_var) if res == 0: # early stop return 0.0 diff --git a/src/pyscipopt/scip.pxd b/src/pyscipopt/scip.pxd index c5053ebba..92e879579 100644 --- a/src/pyscipopt/scip.pxd +++ b/src/pyscipopt/scip.pxd @@ -2284,6 +2284,9 @@ cdef class Variable(Expr): @staticmethod cdef create(SCIP_VAR* scipvar) + cpdef size_t ptr(self) noexcept + cpdef int getIndex(self) noexcept + cdef class Constraint: cdef SCIP_CONS* scip_cons # can be used to store problem data diff --git a/src/pyscipopt/scip.pxi b/src/pyscipopt/scip.pxi index 214b8bad5..019e9eddb 100644 --- a/src/pyscipopt/scip.pxi +++ b/src/pyscipopt/scip.pxi @@ -1562,7 +1562,7 @@ cdef class Variable(Expr): cdef Variable copy(self, bint copy=True): return self - def ptr(self): + cpdef size_t ptr(self) noexcept: return (self.scip_var) def __repr__(self): @@ -1697,7 +1697,7 @@ cdef class Variable(Expr): """ return SCIPvarIsInLP(self.scip_var) - def getIndex(self): + cpdef int getIndex(self) noexcept: """ Retrieve the unique index of the variable.