CPMpy / cpmpy

Constraint Programming and Modeling library in Python, based on numpy, with direct solver access.
Apache License 2.0
206 stars 19 forks source link

from_file seems to create two separate references for the same variable #161

Open rubenkindt opened 1 year ago

rubenkindt commented 1 year ago

solver: solver independent, but found using 'ortools' CPMpy: version v0.9.10 commit 73c233a120c408b94f33fb83aa07e090a1c662cc

As mentioned in #142, the private attributes are not correctly set in this example below, this behavior does not occur when writing down the constraints manually. As Tias Guns meantioned the problem is probably in from_file.

minimized.zip

from cpmpy import *

minimizedFile = "minimized"
m = Model().from_file(minimizedFile)
m.solve(solver="ortools")
print(m.status())
print(m)
correctSet = m.constraints[0].args[0]
wrongSet = m.constraints[1].args[0].args[0].args[0]
print(correctSet.name + " = " + str(correctSet.value())) # grid[3,3] = 7
print(wrongSet.name + " = " + str(wrongSet.value()))     # grid[3,3] = 1

m2 = Model()
i = intvar(lb=0, ub=9, shape=(4,4), name="grid")
m2 += i[3,3] == 7
m2 += ~(~(i[3,3] == 7))
m2.solve(solver="ortools")
print(m2.status())
print(m2)
correctSet = m2.constraints[0].args[0]
wrongSet = m2.constraints[1].args[0].args[0].args[0]
print(correctSet.name + " = " + str(correctSet.value())) # grid[3,3] = 7
print(wrongSet.name + " = " + str(wrongSet.value()))     # grid[3,3] = 7

Bug found while working on my master thesis.

sourdough-bread commented 1 year ago

@rubenkindt Did you perhaps create 2 variables with the same name when you pickled the model? Could you post the original model?

Because unpickling calls the __new__ method to create a class instance. Then it calls the def __setstate__(self, state) to assign values to the instance's attributes. In the model you provided, the __setstate__(self, state) shows that the variable's states are different, which is not possible in the same model. In your case, 2 variables have the same name but they contain different attribute values(verified with different ids).

Object of type (<class 'cpmpy.expressions.variables._IntVarImpl'>) state set to:

{'lb': 1, 'ub': 9, 'name': 'grid[3,3]', '_value': 7}

Object of type (<class 'cpmpy.expressions.variables._IntVarImpl'>) state set to:

{'lb': 1, 'ub': 9, 'name': 'grid[3,3]', '_value': 1}

The code also shows these are 2 distinct variables with different ids:

print(correctSet.name + " = " + str(correctSet.value()), id(correctSet)) # grid[3,3] = 7
print(wrongSet.name + " = " + str(wrongSet.value()), id(wrongSet))     # grid[3,3] = 1

Outputs:

grid[3,3] = 7 4536170144
grid[3,3] = 1 4536172304

Similar to the following example i2[2,2] and i[2,2] have the same hash (name), and are interpreted as 2 different variables (verified with different ids), but when you load them, they will be loaded with the same name.

m2 = Model()
i = intvar(lb=0, ub=9, shape=(4,4), name="grid")
i2 = intvar(lb=0, ub=9, shape=(4,4), name="grid")
m2 += i[2,2] == 7
m2 += ~(~(i2[2,2] == 7))
m2.to_file(mini)
rubenkindt commented 1 year ago

This bug was found using a technique that does not introduce new variables, only creating new constraints from a single file by combining "and"'s and "not"'s. It was then minimized by removing constraints until no removal is possible (with CPMpy's flattening). allModels.zip sudoku_17_hints16650532485163155 was the original seed file used to create new constraints with (using the "and" and "not''s), mutant_0 is the newly created model with those new constraints and minimized is the file we are already working with above after the constraints minimalisation (probably not fully minimized, due to a indexing mistake at the time). I hope this helps