Opened 8 years ago

Closed 7 years ago

Last modified 7 years ago

#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)

trac_10505.patch (1.7 KB) - added by ncohen 7 years ago.

Download all attachments as: .zip

Change History (11)

comment:1 Changed 8 years ago by ncohen

  • Status changed from new to needs_review

comment:2 Changed 7 years ago by dcoudert

  • Reviewers set to David Coudert
  • Status changed from needs_review to needs_info

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 ?

comment:3 Changed 7 years ago by ncohen

  • 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: Changed 7 years ago by dcoudert

  • 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 7 years ago by ncohen

  • 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 7 years ago by ncohen

comment:6 Changed 7 years ago by dcoudert

  • Status changed from needs_review to positive_review

I'm now satisfied with this patch.

comment:7 Changed 7 years ago by jdemeyer

  • Milestone changed from sage-4.7.2 to sage-4.7.3

comment:8 Changed 7 years ago by jdemeyer

  • Merged in set to sage-4.7.3.alpha0
  • Resolution set to fixed
  • Status changed from positive_review to closed

comment:9 Changed 7 years ago by jdemeyer

  • Milestone sage-4.7.3 deleted

Milestone sage-4.7.3 deleted

comment:10 Changed 7 years ago by jdemeyer

  • Merged in changed from sage-4.7.3.alpha0 to sage-4.8.alpha0
  • Milestone set to sage-4.8
Note: See TracTickets for help on using tickets.