Fixes for copying a MIP and its variables

[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]
x_0
sage: q = copy(p)
sage: q[0]
x_0
sage: q[1]
x_1
sage: sage: p.number_of_variables()
2
sage: sage: q.number_of_variables()
1

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.

[mkoeppe]

Any thoughts on this proposal?

[nbruin]

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

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.

[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.

[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
x_0
sage: mip.number_of_variables()
0
sage: mip[0]                  ### This now creates a variable. It prints the same as the result of gen(0), but is different.
x_0
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])
True

[mkoeppe]

Repairing gen() is now #20602.

[mkoeppe]

New commits:

​ed4082f	MIPVariable: Remove unused attributes
​373aff4	MixedIntegerLinearProgram: Documentation fixes
​68305b0	MixedIntegerLinearProgram.__getitem__: Simplify attribute access
​f696ba1	MIPVariable.copy_for_mip: New
​8edd4d8	MixedIntegerLinearProgram.__copy__: Copy the default MIP variable.
​03b34b1	MixedIntegerLinearProgram.show: Remove unused code
​fdd5c18	MixedIntegerLinearProgram: Documentation fixes

[mkoeppe]

Needs review.

[dimpase]

Shouldn't copy_for_mip actually become copy ? Or there is a reason to keep copy as well?

[mkoeppe]

Currently there is no copy. It seems an unusual operation to make a copy of a MIPVariable for the same problem.

[git]

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

​f59295c	MIPVariable: Remove unused attributes
​67d8af3	MixedIntegerLinearProgram: Documentation fixes
​f230df9	MixedIntegerLinearProgram.__getitem__: Simplify attribute access
​2fb5134	MIPVariable.copy_for_mip: New
​689becb	MixedIntegerLinearProgram.__copy__: Copy the default MIP variable.
​4927701	MixedIntegerLinearProgram.show: Remove unused code
​af80960	MixedIntegerLinearProgram: Documentation fixes
​dff27e5	MIPVariable.__copy__, __deepcopy_: New

[mkoeppe]

But anyway I made __copy__ and __deepcopy methods. (& rebased)

[mkoeppe]

ping?

[dimpase]

OK, looks good to me.

[vbraun]

Reviewer name

[dimpase]

I will never learn the drill :-)