From b49be89dc413a249800be17b0d0eebf4e034790c Mon Sep 17 00:00:00 2001 From: Haroldo Santos Date: Thu, 24 Sep 2026 15:08:18 -0400 Subject: [PATCH] Fix silent coefficient loss for duplicate variables in LinExpr constructor Issue #396 / PR #397: constructing a LinExpr directly with a variables list containing the same Var more than once (e.g. via Var.__add__, Var.__sub__, Var.__eq__/__le__/__ge__ on identical objects, or manual LinExpr(variables=..., coeffs=...) calls) silently kept only the last coefficient because dict(zip(...)) overwrites duplicate keys. PR #397's approach patches this only for the specific case where a Var is combined with itself in a handful of dunder methods, and explicitly does not fix direct constructor calls with 3+ duplicate entries (see its commented-out test case). This fixes the root cause in the LinExpr constructor itself, covering all call sites. To avoid regressing performance for the common duplicate-free case, a cheap len(variables) == len(set(variables)) check picks the fast dict(zip(...)) path when there are no duplicates, falling back to an accumulating loop only when needed. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- mip/entities.py | 12 +++++++++++- test/mip_test.py | 38 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 49 insertions(+), 1 deletion(-) diff --git a/mip/entities.py b/mip/entities.py index e3c6d5c2..02f6232a 100644 --- a/mip/entities.py +++ b/mip/entities.py @@ -89,7 +89,17 @@ def __init__( "You should pass eiter 'expr' or 'variables and coeffs' to the" "constructor, not the three simultaneously." ) - self.__expr = dict(zip(variables, coeffs)) + # dict(zip(...)) silently drops all but the last coefficient when + # the same variable appears more than once, so fall back to an + # accumulating loop only when duplicates are actually present. + # This keeps the common (duplicate-free) case as fast as before. + if len(variables) == len(set(variables)): + self.__expr = dict(zip(variables, coeffs)) + else: + expr_dict = {} # type: dict[mip.Var, mip.Numeric] + for var, coeff in zip(variables, coeffs): + expr_dict[var] = expr_dict.get(var, 0) + coeff + self.__expr = expr_dict elif expr is not None: self.__expr = expr.copy() diff --git a/test/mip_test.py b/test/mip_test.py index 93a8ae9e..58c2573b 100644 --- a/test/mip_test.py +++ b/test/mip_test.py @@ -7,6 +7,7 @@ import mip.gurobi import mip.highs from mip import Model, xsum, OptimizationStatus, MAXIMIZE, BINARY, INTEGER +from mip.entities import LinExpr from mip import ConstrsGenerator, CutPool, maximize, CBC, GUROBI, HIGHS, Column, Constr from os import environ from util import skip_on, has_gurobi_license @@ -588,6 +589,43 @@ def test_obj_const2(self, solver: str): assert model.objective_const == 1 +@skip_on(NotImplementedError) +@pytest.mark.parametrize("solver", SOLVERS) +@pytest.mark.parametrize( + "constraint, lb, ub", + [ + (lambda x: x + x >= 3, 1, 2), + (lambda x: x - x >= 0, 1, 2), + (lambda x: x == x, 1, 2), + (lambda x: x >= x, 1, 2), + (lambda x: x <= x, -2, -1), + (lambda x: LinExpr([x, x, x], [2, -1, -1], sense="="), 1, 2), + ], +) +def test_identical_vars(solver: str, constraint, lb, ub): + """Try if constraints are correctly added when variables are identical""" + m = Model(solver_name=solver) + x = m.add_var(name="x", lb=lb, ub=ub, obj=1) + + m.add_constr(constraint(x)) + + m.optimize() + assert m.status == OptimizationStatus.OPTIMAL + assert lb - TOL <= x.x <= ub + TOL + + +def test_linexpr_duplicate_variables_constructor(): + """LinExpr constructed directly with duplicate variables should sum + coefficients instead of silently dropping earlier ones (issue #396).""" + m = Model() + x = m.add_var(name="x") + y = m.add_var(name="y") + + expr = LinExpr([x, y, x], [1, 2, 3]) + assert expr.expr[x] == 4 + assert expr.expr[y] == 2 + + @skip_on(NotImplementedError) @pytest.mark.parametrize("val", range(1, 4)) @pytest.mark.parametrize("solver", SOLVERS)