Skip to content

Conversation

ItsNiklas
Copy link

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
Copy link

CLAassistant commented Nov 11, 2024

CLA assistant check
All committers have signed the CLA.

@simberaj
Copy link

@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
Copy link
Member

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Incorrect handling of constraints with identical variables causing infeasible models
4 participants