Opened 3 months ago

Closed 2 months ago

#27774 closed enhancement (fixed)

py3: fix various issues in cplex backend

Reported by: dcoudert Owned by:
Priority: major Milestone: sage-8.8
Component: numerical Keywords: py3, numerical
Cc: slabbe Merged in:
Authors: David Coudert Reviewers: Travis Scrimshaw
Report Upstream: N/A Work issues:
Branch: c408bc0 (Commits) Commit: c408bc0c78934d27d681f986a3ae04f3dfaab60d
Dependencies: Stopgaps:

Description

Apart from the problems reported in #27773, several doctests are failing in Python 3 in src/sage/numerical/backends/cplex_backend.pyx due to:

  • bytes versus str
  • passing iterator (zip) instead of list in add_linear_constraint

and some issues are already here with Python 2:

  • int(CPX_INFBOUND) give 0 while CPX_INFBOUND == 1E+20. Hence, some doctests are failing. Actually, the int(...) is not needed
  • with a integer variable, get_variable_value return 2.0 in py2 and 2 in py3 as round has a different behavior. We unify the behavior using int(...).

Change History (7)

comment:1 Changed 3 months ago by dcoudert

  • Branch set to public/numerical/27774_various_issues_with_cplex
  • Commit set to 61bf0bb2612b3cee5c822512a28361eec9f1ba36
  • Status changed from new to needs_review

New commits:

61bf0bbtrac #27774: fix various issues in cplex backend

comment:2 Changed 3 months ago by dcoudert

  • Cc slabbe added

similar issues may also occur with gurobi...

comment:3 Changed 2 months ago by tscrim

While I am not strictly opposed to these sorts of changes:

-            sage: p.add_linear_constraint( zip(range(5), range(5)), 1.0, 1.0, name='foo') # optional - CPLEX
+            sage: p.add_linear_constraint([(i, i) for i in range(5)], 1.0, 1.0, name='foo') # optional - CPLEX

I think it would be better to have add_linear_constraint handle these types of inputs.

comment:4 Changed 2 months ago by git

  • Commit changed from 61bf0bb2612b3cee5c822512a28361eec9f1ba36 to c408bc0c78934d27d681f986a3ae04f3dfaab60d

Branch pushed to git repo; I updated commit sha1. New commits:

2667918trac #27774: Merged with 8.8.beta5
c408bc0trac #27774: make add_linear_constraint accept iterator

comment:5 Changed 2 months ago by dcoudert

I did a change in all backends in which coefficients was iterated several times. May be there is a smarter way to do it, but at least this one is working.

To show that it's working, I ensured that in all the modified files, the doctest is with zip(...) and not list(zip(...)).

comment:6 Changed 2 months ago by tscrim

  • Reviewers set to Travis Scrimshaw
  • Status changed from needs_review to positive_review

Thank you. I think this is a much better solution (and now the behavior also matches the docstring).

comment:7 Changed 2 months ago by vbraun

  • Branch changed from public/numerical/27774_various_issues_with_cplex to c408bc0c78934d27d681f986a3ae04f3dfaab60d
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.