Opened 6 years ago

Last modified 6 years ago

#20461 closed defect

Fixes for copying a MIP and its variables — at Version 8

Reported by: mkoeppe Owned by:
Priority: major Milestone: sage-7.3
Component: numerical Keywords: lp
Cc: dimpase, vdelecroix, jdemeyer, nbruin, chapoton Merged in:
Authors: Matthias Koeppe Reviewers:
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: Stopgaps:

Status badges

Description (last modified by mkoeppe)

Even after merging #20414, both copy and (the identical) deepcopy have weird semantics in relation to the MIPVariables in a problems -- because a MIP does not have an API to gain access to its variables; and trying to use the old MIPVariables with the copy is questionable and definitely broken if one creates new components of this variable, see #15159, #19523. Same when there's no explicit MIPVariable and only the _default_mipvariable is used via the problem's __getitem__ method:

sage: sage: p = MixedIntegerLinearProgram()
sage: p[0]
sage: q = copy(p)
sage: q[0]
sage: q[1]
sage: sage: p.number_of_variables()
sage: sage: q.number_of_variables()

Here's a possible fix without having to change the whole design:

  • a new MixedIntegerLinearProgram.copy method that takes a names keyword argument, enabling this operation:
    sage: p.<x,y> = MixedIntegerLinearProgram()
    sage: q.<newx,newy> = p.copy()

and the less magical syntax

sage: q, newx, newy = p.copy([x, y])
  • copy and __copy__ (and __deepcopy__) should make a copy of _default_variable (this requires writing a __copy__ method for MIPVariable)
  • if MixedIntegerLinearProgram.new_variable has been called, it should set a flag and then if __copy__ (or __deepcopy__) are called, it should display a warning (deprecation??) and refer the user to the new copy method.

In this ticket, we take care of the default MIP variable. #20657 will take care of the variables created with new_variable.

Change History (8)

comment:1 Changed 6 years ago by mkoeppe

  • Description modified (diff)

comment:2 Changed 6 years ago by mkoeppe

Any thoughts on this proposal?

comment:3 Changed 6 years ago by mkoeppe

  • Description modified (diff)

comment:4 Changed 6 years ago by nbruin

sage: p = MixedIntegerLinearProgram()
sage: x = p.new_variable(nonnegative=True)
sage: x[0]
sage: x[1]
sage: p[0] #I don't understand this
sage: p[1]

But if we actually use the "generator" interface things seem OK:

sage: [p.gen(i) for i in [0..3]]
[x_0, x_1, x_2, x_3]

so I would suggest to just make sure that p.gen(...) works properly and indeed gives a variable of the appropriate MIP object. Then there IS an interface to get access to its variables (and really, it seems to be largely in place already) and then simply that can be used.

It looks like a variable on p is uniquely determined by its index (certainly its print name is), and p.new_variable seems just a funny way of obtaining such indices.

The rest of the variable data is all hanging off internal tables, and via the "is_variable_*" methods you can obtain the type of the variables.

I think you first need to make sure that all of that is accessible (and copied appropriately, but I guess that's taken care of now) and then you can think about what to do with this "new_variable" business.

comment:5 Changed 6 years ago by mkoeppe

If one just uses gen(). then everything already works as expected.

What is broken with copy is the funny stuff involving MIPVariable. This is what this ticket addresses.

comment:6 Changed 6 years ago by mkoeppe

Actually I spoke too fast, it seems that gen() does not do anything useful.

sage: mip = MixedIntegerLinearProgram(solver='GLPK')
sage: mip.gen(0)           ### Names a variable, but does not create it in the backend
sage: mip.number_of_variables()
sage: mip[0]                  ### This now creates a variable. It prints the same as the result of gen(0), but is different.
sage: mip.get_values(mip.gen(0))
TypeError: Not a MIPVariable: x_0
sage: mip.is_real(mip.gen(0))
KeyError: x_0
sage: mip.is_real(mip[0])

comment:7 Changed 6 years ago by mkoeppe

Repairing gen() is now #20602.

comment:8 Changed 6 years ago by mkoeppe

  • Authors set to Matthias Koeppe
  • Description modified (diff)
  • Milestone changed from sage-7.2 to sage-7.3
Note: See TracTickets for help on using tickets.