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: |
Description (last modified by )
Even after merging #20414, both copy
and (the identical) deepcopy
have weird semantics in relation to the MIPVariable
s in a problems -- because a MIP does not have an API to gain access to its variables; and trying to use the old MIPVariable
s 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 anames
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 forMIPVariable
)- 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 newcopy
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
- Description modified (diff)
comment:2 Changed 6 years ago by
comment:3 Changed 6 years ago by
- Description modified (diff)
comment:4 Changed 6 years ago by
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.
comment:5 Changed 6 years ago by
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
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
comment:7 Changed 6 years ago by
Repairing gen()
is now #20602.
comment:8 Changed 6 years ago by
- Description modified (diff)
- Milestone changed from sage-7.2 to sage-7.3
Any thoughts on this proposal?