#10505 closed defect (fixed)
Round values returned by CPLEX when the variable's type is integer/binary
Reported by: | ncohen | Owned by: | ncohen |
---|---|---|---|
Priority: | major | Milestone: | sage-4.8 |
Component: | linear programming | Keywords: | |
Cc: | Merged in: | sage-4.8.alpha0 | |
Authors: | Nathann Cohen | Reviewers: | David Coudert |
Report Upstream: | N/A | Work issues: | |
Branch: | Commit: | ||
Dependencies: | Stopgaps: |
Description
CPLEX is a funny guy who does not understand that 1.546846847 e-17 is not a proper value for a binary variable. As I often forgot to replace test == 1
by
> .5
and as it is anyway what CPLEX should return, this patch rounds values given to the variables before returning them when it is sound. It amounts to copying two lines from
is_variable_continuous
in the
get_variable_value
method, and doctests pass better with CPLEX (which is not the case with the new patch for TSP #10497)
Nathann
Attachments (1)
Change History (11)
comment:1 Changed 10 years ago by
- Status changed from new to needs_review
comment:2 Changed 9 years ago by
- Reviewers set to David Coudert
- Status changed from needs_review to needs_info
comment:3 Changed 9 years ago by
- Status changed from needs_info to needs_review
Oops... You are right !
The thing is that casting it to Integer would mean having to create a new object for each returned variable, even when using the code from the C level. And casting it to a Python int would mean that you can not divide is safely anymore (1/2 becomes 0).
What do you think of letting it stay like that ? The goal was really to be able to test (variable == 0) without fearing that variable (declared as binary in the LP) would have 0.0000001 as a value.
We can still *really* cast the values to integer in situations where the user expect it, like in #11367 which just got reviewed.
While I was at it, though, I noticed a mistake in the documentation generated from the mip.pyx file : the first line contained a math equation which was not properly written.. I just updated the patch to fix it ! :-)
Nathann
comment:4 follow-up: ↓ 5 Changed 9 years ago by
- Status changed from needs_review to needs_info
OK, you can live it like this, but it should be clearly indicated in the documentation to avoid confusion. Users can cast afterwards depending of their needs.
Concerning the documentation, I don't see any improvement. In fact, most of the math equations are not properly displayed in the page. I see only the latex code (e.g., w_i \in \mathbb{Z}).
comment:5 in reply to: ↑ 4 Changed 9 years ago by
- Status changed from needs_info to needs_review
OK, you can live it like this, but it should be clearly indicated in the documentation to avoid confusion. Users can cast afterwards depending of their needs.
Patch updated. I added a note in the documentation of MixedIntegerLinearProgram?.get_values
.
Concerning the documentation, I don't see any improvement. In fact, most of the math equations are not properly displayed in the page. I see only the latex code (e.g., w_i \in \mathbb{Z}).
Are you talking of the HTML documentation ? The LaTeX formula will of course never appear in the console ! In my version of it, I find nothing wrong with the others formulas contained in this page. O_o
Nathann
Changed 9 years ago by
comment:6 Changed 9 years ago by
- Status changed from needs_review to positive_review
I'm now satisfied with this patch.
comment:7 Changed 9 years ago by
- Milestone changed from sage-4.7.2 to sage-4.7.3
comment:8 Changed 9 years ago by
- Merged in set to sage-4.7.3.alpha0
- Resolution set to fixed
- Status changed from positive_review to closed
comment:10 Changed 9 years ago by
- Merged in changed from sage-4.7.3.alpha0 to sage-4.8.alpha0
- Milestone set to sage-4.8
The returned value is not of integer type (2.0 instead of 2). Should we cast it to Integer, or int, or leave it like that ?