From 0285923174c084a75dc2fbd004f801d6ae1e57c1 Mon Sep 17 00:00:00 2001 From: 40% Date: Wed, 4 Feb 2026 11:15:20 +0800 Subject: [PATCH 1/9] Deprecated `Term.ptrtuple` Remove the separate ptrtuple from Term and switch sorting/merging to use each Variable's hash instead of a ptr() tuple. Compute Term.hashval from vartuple directly and implement a robust __eq__ that checks identity, type, length, hashval and then element-wise variable identity. In Variable, expose __hash__ returning the underlying scip_var pointer and make ptr() delegate to that hash. Update the .pyi stub to drop the now-removed ptrtuple. These changes simplify Term state and unify ordering/identity on Variable hashes (pointer-based). --- src/pyscipopt/expr.pxi | 32 +++++++++++++++++++++++--------- src/pyscipopt/scip.pxi | 6 ++++-- src/pyscipopt/scip.pyi | 1 - 3 files changed, 27 insertions(+), 12 deletions(-) diff --git a/src/pyscipopt/expr.pxi b/src/pyscipopt/expr.pxi index f782a46da..15cafb035 100644 --- a/src/pyscipopt/expr.pxi +++ b/src/pyscipopt/expr.pxi @@ -104,13 +104,11 @@ cdef class Term: '''This is a monomial term''' cdef readonly tuple vartuple - cdef readonly tuple ptrtuple cdef Py_ssize_t hashval def __init__(self, *vartuple: Variable): - self.vartuple = tuple(sorted(vartuple, key=lambda v: v.ptr())) - self.ptrtuple = tuple(v.ptr() for v in self.vartuple) - self.hashval = hash(self.ptrtuple) + self.vartuple = tuple(sorted(vartuple, key=hash)) + self.hashval = hash(self.vartuple) def __getitem__(self, idx): return self.vartuple[idx] @@ -118,8 +116,25 @@ cdef class Term: def __hash__(self) -> Py_ssize_t: return self.hashval - def __eq__(self, other: Term): - return self.ptrtuple == other.ptrtuple + def __eq__(self, other: Term) -> bool: + if self is other: + return True + if type(other) is not Term: + return False + + cdef int n = len(self) + cdef Term _other = other + if n != len(_other) or self.hashval != _other.hashval: + return False + + cdef int i + cdef Variable var1, var2 + for i in range(n): + var1 = PyTuple_GET_ITEM(self.vartuple, i) + var2 = PyTuple_GET_ITEM(_other.vartuple, i) + if var1 is not var2: + return False + return True def __len__(self): return len(self.vartuple) @@ -138,7 +153,7 @@ cdef class Term: while i < n1 and j < n2: var1 = PyTuple_GET_ITEM(self.vartuple, i) var2 = PyTuple_GET_ITEM(other.vartuple, j) - if var1.ptr() <= var2.ptr(): + if hash(var1) <= hash(var2): vartuple[k] = var1 i += 1 else: @@ -156,8 +171,7 @@ cdef class Term: cdef Term res = Term.__new__(Term) res.vartuple = tuple(vartuple) - res.ptrtuple = tuple(v.ptr() for v in res.vartuple) - res.hashval = hash(res.ptrtuple) + res.hashval = hash(res.vartuple) return res def __repr__(self): diff --git a/src/pyscipopt/scip.pxi b/src/pyscipopt/scip.pxi index 9c21942bd..136b89013 100644 --- a/src/pyscipopt/scip.pxi +++ b/src/pyscipopt/scip.pxi @@ -1558,10 +1558,12 @@ cdef class Variable(Expr): cname = bytes( SCIPvarGetName(self.scip_var) ) return cname.decode('utf-8') - def ptr(self): - """ """ + def __hash__(self) -> size_t: return (self.scip_var) + def ptr(self) -> size_t: + return self.__hash__() + def __repr__(self): return self.name diff --git a/src/pyscipopt/scip.pyi b/src/pyscipopt/scip.pyi index e8bbc46d5..b8ad35a52 100644 --- a/src/pyscipopt/scip.pyi +++ b/src/pyscipopt/scip.pyi @@ -2190,7 +2190,6 @@ class SumExpr(GenExpr): @disjoint_base class Term: - ptrtuple: Incomplete vartuple: Incomplete def __init__(self, *vartuple: Incomplete) -> None: ... def __mul__(self, other: Term) -> Term: ... From 92a16ba2c194bb94bf0ffc78995ae69c176c7691 Mon Sep 17 00:00:00 2001 From: 40% Date: Wed, 4 Feb 2026 13:26:24 +0800 Subject: [PATCH 2/9] Changelog: Term __eq__ speedup, ptrtuple deprecated Update CHANGELOG.md to record a performance improvement: Term.__eq__ was sped up using the C-level API, and to note that Term.ptrtuple is deprecated to enable Term memory optimizations. --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index c41f56a78..51634de0a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -32,7 +32,9 @@ - changed `addConsNode()` and `addConsLocal()` to mirror `addCons()` and accept `ExprCons` instead of `Constraint` - Improved `chgReoptObjective()` performance - Return itself for `abs` to `UnaryExpr(Operator.fabs)` +- Speed up `Term.__eq__` via the C-level API ### Removed +- Deprecated `Term.ptrtuple` for optimizing `Term` memory usage ## 6.0.0 - 2025.11.28 ### Added From e0fdc9cfdfcbfff51680b436feeae9bc2fd27591 Mon Sep 17 00:00:00 2001 From: 40% Date: Wed, 4 Feb 2026 13:44:40 +0800 Subject: [PATCH 3/9] Add rich comparison to Variable Implement __richcmp__ in Variable (src/pyscipopt/scip.pxi) to delegate rich-comparison operations to _expr_richcmp, enabling Python comparison operators between Variable and other Expr objects. --- src/pyscipopt/scip.pxi | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/pyscipopt/scip.pxi b/src/pyscipopt/scip.pxi index 136b89013..6517250a5 100644 --- a/src/pyscipopt/scip.pxi +++ b/src/pyscipopt/scip.pxi @@ -1564,6 +1564,9 @@ cdef class Variable(Expr): def ptr(self) -> size_t: return self.__hash__() + def __richcmp__(self, other, int op): + return _expr_richcmp(self, other, op) + def __repr__(self): return self.name From 6b0788f12e639133d735c4ded4b0c9c4c7a5dcc0 Mon Sep 17 00:00:00 2001 From: 40% Date: Wed, 4 Feb 2026 13:45:35 +0800 Subject: [PATCH 4/9] tests: add term equality tests for expressions Import quickprod and add test_term_eq to verify term equality/inequality behavior. The new test creates terms from quickprod over a matrix variable and checks that identical terms compare equal, different terms (even of same length) compare unequal, terms of different lengths are unequal, and comparison with a different type returns False. This ensures Expr term __eq__ semantics are correct. --- tests/test_expr.py | 23 ++++++++++++++++++++++- 1 file changed, 22 insertions(+), 1 deletion(-) diff --git a/tests/test_expr.py b/tests/test_expr.py index a4e739b76..aa13d8b13 100644 --- a/tests/test_expr.py +++ b/tests/test_expr.py @@ -2,7 +2,7 @@ import pytest -from pyscipopt import Model, sqrt, log, exp, sin, cos +from pyscipopt import Model, sqrt, log, exp, sin, cos, quickprod from pyscipopt.scip import Expr, GenExpr, ExprCons, CONST @@ -244,3 +244,24 @@ def test_abs_abs_expr(): # should print abs(x) not abs(abs(x)) assert str(abs(abs(x))) == str(abs(x)) + + +def test_term_eq(): + m = Model() + + x = m.addMatrixVar(1000) + y = m.addVar() + z = m.addVar() + + e1 = quickprod(x.flat) + e2 = quickprod(x.flat) + t1 = next(iter(e1)) + t2 = next(iter(e2)) + t3 = next(iter(e1 * y)) + t4 = next(iter(e2 * z)) + + assert t1 == t1 # same term + assert t1 == t2 # same term + assert t3 != t4 # same length, but different term + assert t1 != t3 # different length + assert t1 != "not a term" # different type From 09decde31fa6aef812ef4fc9fba57a2d7709ac7d Mon Sep 17 00:00:00 2001 From: 40% Date: Wed, 4 Feb 2026 13:47:06 +0800 Subject: [PATCH 5/9] Use hash comparison for variable equality in Term Replace identity check with hash comparison when comparing variables in Term equality. Using hash(var) != hash(var) instead of `is not` avoids false negatives when two distinct Python wrapper objects represent the same underlying variable, ensuring Term comparisons treat equivalent variables as equal. This relies on Variable.__hash__ to reflect variable identity. --- src/pyscipopt/expr.pxi | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/pyscipopt/expr.pxi b/src/pyscipopt/expr.pxi index 15cafb035..ab89ba202 100644 --- a/src/pyscipopt/expr.pxi +++ b/src/pyscipopt/expr.pxi @@ -132,7 +132,7 @@ cdef class Term: for i in range(n): var1 = PyTuple_GET_ITEM(self.vartuple, i) var2 = PyTuple_GET_ITEM(_other.vartuple, i) - if var1 is not var2: + if hash(var1) != hash(var2): return False return True From 43cdc51471e8ac27699167e8f670b70924960fb3 Mon Sep 17 00:00:00 2001 From: 40% Date: Wed, 4 Feb 2026 13:48:24 +0800 Subject: [PATCH 6/9] Apply AI suggestion Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 51634de0a..cca533149 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -34,7 +34,7 @@ - Return itself for `abs` to `UnaryExpr(Operator.fabs)` - Speed up `Term.__eq__` via the C-level API ### Removed -- Deprecated `Term.ptrtuple` for optimizing `Term` memory usage +- Removed `Term.ptrtuple` to optimize `Term` memory usage ## 6.0.0 - 2025.11.28 ### Added From 6c69514efbaf067fe7b9503b52d0f499af47c7c4 Mon Sep 17 00:00:00 2001 From: 40% Date: Wed, 4 Feb 2026 13:58:51 +0800 Subject: [PATCH 7/9] Improve Term.__eq__ type check Remove the strict Cython annotation on the __eq__ parameter and replace the Python-level type() check with a C-level Py_TYPE check. Also normalize the identity check order. These changes make the equality method more robust and slightly faster by avoiding a Python call for type comparison and allowing broader input types during runtime. --- src/pyscipopt/expr.pxi | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/pyscipopt/expr.pxi b/src/pyscipopt/expr.pxi index ab89ba202..fe9fa6e49 100644 --- a/src/pyscipopt/expr.pxi +++ b/src/pyscipopt/expr.pxi @@ -116,10 +116,10 @@ cdef class Term: def __hash__(self) -> Py_ssize_t: return self.hashval - def __eq__(self, other: Term) -> bool: - if self is other: + def __eq__(self, other) -> bool: + if other is self: return True - if type(other) is not Term: + if Py_TYPE(other) is not Term: return False cdef int n = len(self) From 266e65d6ef500869b2ef242a6b570d20a96ac187 Mon Sep 17 00:00:00 2001 From: 40% Date: Thu, 5 Feb 2026 18:23:29 +0800 Subject: [PATCH 8/9] Promote Term changes to top of CHANGELOG Move two entries about Term optimizations out of the 6.1.0 section and into the top-level changelog sections: "Speed up Term.__eq__ via the C-level API" (Changed) and "Removed Term.ptrtuple to optimize Term memory usage" (Removed). This removes duplicate lines from 6.1.0 and surfaces these changes for the upcoming release. --- CHANGELOG.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2caede3fa..3a2dfc46f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,7 +4,9 @@ ### Added ### Fixed ### Changed +- Speed up `Term.__eq__` via the C-level API ### Removed +- Removed `Term.ptrtuple` to optimize `Term` memory usage ## 6.1.0 - 2026.01.31 ### Added @@ -39,9 +41,7 @@ - changed `addConsNode()` and `addConsLocal()` to mirror `addCons()` and accept `ExprCons` instead of `Constraint` - Improved `chgReoptObjective()` performance - Return itself for `abs` to `UnaryExpr(Operator.fabs)` -- Speed up `Term.__eq__` via the C-level API ### Removed -- Removed `Term.ptrtuple` to optimize `Term` memory usage ## 6.0.0 - 2025.11.28 ### Added From 3294b9a58a17a2b6e652b92719e2ea3796c3462a Mon Sep 17 00:00:00 2001 From: 40% Date: Thu, 5 Feb 2026 23:52:27 +0800 Subject: [PATCH 9/9] Remove return type annotations in scip.pxi warning: src\pyscipopt\scip.pxi:1564:21: Unknown type declaration 'Py_ssize_t' in annotation, ignoring --- src/pyscipopt/scip.pxi | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/pyscipopt/scip.pxi b/src/pyscipopt/scip.pxi index 6517250a5..d1fd20021 100644 --- a/src/pyscipopt/scip.pxi +++ b/src/pyscipopt/scip.pxi @@ -1558,10 +1558,10 @@ cdef class Variable(Expr): cname = bytes( SCIPvarGetName(self.scip_var) ) return cname.decode('utf-8') - def __hash__(self) -> size_t: + def __hash__(self): return (self.scip_var) - def ptr(self) -> size_t: + def ptr(self): return self.__hash__() def __richcmp__(self, other, int op):