Duplicate Variables Bandaid

[ItsNiklas]

Attempting to fix #396. If we have two identical objects in the Var overrides, the previous code would allow the LinExpr constructor to void anything but the last coefficient.

I also added the relevant tests, which now pass. The last constraint would fail, because I only modified the Var functions, not the constructor itself.

---

Alternative:

diff --git a/mip/entities.py b/mip/entities.py
index fcd3ecd..71091b5 100644
--- a/mip/entities.py
+++ b/mip/entities.py
@@ -92,7 +92,8 @@ class LinExpr:
                     "You should pass eiter 'expr' or 'variables and coeffs' to the"
                     "constructor, not the three simultaneously."
                 )
-            self.__expr = dict(zip(variables, coeffs))
+            for var, coef in zip(variables, coeffs):
+                self.add_var(var, coef)
 
         elif expr is not None:
             self.__expr = expr.copy()

However, this would tank a performance slightly if users create the LinExpr manually. For this benchmark, the using_lists method runs in 0.71s vs 0.33s for n=2000. Considering the last change was made due to performance reasons, I did not want to revert this right away, but feel free to approve this.

[CLAassistant]

All committers have signed the CLA.

[simberaj]

@DominikPeters @tkralphs could we get this merged and released? This is a critical bug affecting my workflow. I'd chip in but I don't have the necessary rights.

[tkralphs]

If by "released", you mean released on Pypi, then I can't currently do that. I'm trying to get access to the project on Pypi from the previous maintainers.

[simberaj]

Ok, much appreciated! Any way I could help you with that?

[tkralphs]

I now got access to Pypi. It would be great is if we could get all of the tests passing. There are a lot of failures happening that are related to issues with the HiGHs interface that have been fixed but not merged. I am hoping @rschwarz will make a PR (see this comment).

It would also be great if we could have a Github action that uploads to Pypi automatically upon tagging a release, as is done in, e.g., CyLP.

[rschwarz]

OK, I'll try to prepare the PR tomorrow.

UPDATE: see here: #418

[chrjabs]

Is there an chance of this getting merged any time soon? There have been multiple releases now, so it would be great to have this fixed.

[h-g-s]

Thank you for finding and documenting this bug, and for adding the first tests.

I confirmed that the problem is still present in the current master branch.

I opened a suggested fix in #429. Instead of patching only the Var operator methods, it fixes the root cause in the LinExpr constructor. This also handles direct constructions such as LinExpr([x, x, x], [2, -1, -1]), where repeated coefficients were being lost.

The normal case keeps the fast dictionary path, so the performance impact should be small. The added tests pass: 68 passed, 1 skipped.

Could you please check whether these cases fix the problem you reported? If so, this approach should cover both your original case and the broader direct-constructor case.

[chrjabs]

Yes, your PR resolved this, thanks.