Opened 7 months ago

Closed 2 weeks ago

#27790 closed defect (fixed)

compatibility with cplex 12.9

Reported by: dcoudert Owned by:
Priority: major Milestone: sage-9.0
Component: numerical Keywords:
Cc: asante Merged in:
Authors: David Coudert Reviewers: Sébastien Labbé
Report Upstream: N/A Work issues:
Branch: 0881f5b (Commits) Commit: 0881f5b559cfabb53c8eac4d081bef893352cfa5
Dependencies: #28382 Stopgaps:

Description

Method CPXsetlogfile has been deprecated in cplex 12.8 and removed from cplex 12.9. We must instead use CPXsetlogfilename, introduced in 12.8.

Change History (33)

comment:1 Changed 7 months ago by dcoudert

  • Branch set to public/numerical/27790_cplex_logfilename
  • Commit set to 62a189900d82cb2a35ae369872950b9a01365083
  • Status changed from new to needs_review

This patch works with versions >= 12.8. However, we loose compatibility with versions < 12.8.

To maintain compatibility, we must be able to import CPXsetlogfile only if version < 12.8 and CPXsetlogfilename if version >= 12.8. Then, we can use CPXversionnumber to adjust usage.

I don't know how to do that.


New commits:

62a1899trac #27790: use CPXsetlogfilename

comment:2 follow-up: Changed 7 months ago by vdelecroix

  1. merge failure
  1. A possibility to make it work for both CPLEX versions, you can find out the CPLEX version from the Sage setup.py script and set a Cython constant so that you can use Cython conditional statements.

comment:3 Changed 7 months ago by git

  • Commit changed from 62a189900d82cb2a35ae369872950b9a01365083 to 61e518719f22fe5d1456d1a2883df9405f59c8aa

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

61e5187trac #27790: fix merge conflict with 8.8.beta6

comment:4 in reply to: ↑ 2 ; follow-up: Changed 7 months ago by dcoudert

Replying to vdelecroix:

  1. merge failure

fixed

  1. A possibility to make it work for both CPLEX versions, you can find out the CPLEX version from the Sage setup.py script and set a Cython constant so that you can use Cython conditional statements.

Not sure how to do that and where to put the code (setup.py, module_list.py ?).

Somehow, we want to extract line #define CPX_VERSION 12080000 from cpxconst.h, extract the number and use it for conditional compilation.

comment:5 in reply to: ↑ 4 Changed 7 months ago by vdelecroix

Replying to dcoudert:

Replying to vdelecroix:

  1. A possibility to make it work for both CPLEX versions, you can find out the CPLEX version from the Sage setup.py script and set a Cython constant so that you can use Cython conditional statements.

Not sure how to do that and where to put the code (setup.py, module_list.py ?).

It would have been easier with an optional package and a proper spkg-install... I don't know where to put it. These optional Cython modules are not very handy.

Somehow, we want to extract line #define CPX_VERSION 12080000 from cpxconst.h, extract the number and use it for conditional compilation.

I don't think that can be done directly from Cython. Here is a suggestion (similar to what is done in PyNormaliz)

  • run a configure script (or whatever) to obtain the value of CPX_VERSION and write a line "CPX_VERSION=12080000" in a file cplex_config.py
  • in the setup.py script, use the clpex_config.py to define a compile time constant and branch appropriately in the Cython code

If you find an easier way to access the CPX_VERSION macro directly from a .pyx file that would be great!

comment:6 Changed 7 months ago by dcoudert

An idea to make the process transparent to users is to set the version number in module_list.py. I can get the version number like that, but I don't know how to use it with OptionalExtension.

sage: if os.path.exists(SAGE_LOCAL + '/include/cpxconst.h'):
....:     CPX_VERSION = subprocess.check_output(" grep '#define CPX_VERSION ' $SAGE_LOCAL/include/cpxconst.h | grep -o -E '[0-9]+' ", shell=True).strip()
....:     
sage: CPX_VERSION
'12080000'

comment:7 Changed 7 months ago by vdelecroix

You can try the following (that works for normal extensions)

OptionalExtension(...,
   define_macros= [('CPX_VERSION', 12080000)]   # computed value
)

and the in the Cython code (with upper case)

IF CPX_VERSION >= 12080000:
    check( CPXsetlogfilename(self.env, NULL, NULL) )
ELSE:
    check( CPXsetlogfile(self.env, NULL) )

Since both versions of the code only use the self.env parameter you might want to isolate the IF/ELSE statement in a short cdef function.

comment:8 Changed 7 months ago by dcoudert

This is roughly what I already tried: use define_macros, and in cplex_backend.pxd use the macro to conditionally import methods from cplex.h, but I always get

Error compiling Cython file:
------------------------------------------------------------
...
     int CPX_PARAM_SOLNPOOLGAP = 2105
     int CPX_PARAM_SOLNPOOLINTENSITY = 2107
     int CPX_MAX = -1
     int CPX_MIN = 1

IF CPX_VERSION >= 12090000:
  ^
------------------------------------------------------------

sage/numerical/backends/cplex_backend.pxd:217:3: Compile-time name 'CPX_VERSION' not defined

comment:9 Changed 7 months ago by vdelecroix

try compile_time_env instead of define_macros (see cython thread)

comment:10 Changed 7 months ago by git

  • Commit changed from 61e518719f22fe5d1456d1a2883df9405f59c8aa to 72e1ea0439875af0d45840767bd0cdf3b4761242

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

72e1ea0trac #27790: try to use compile time parameter

comment:11 Changed 7 months ago by dcoudert

I don't understand how I can use compile_time_env.

I pushed my current trial that is not working as can be seen from the patchbot.

comment:12 Changed 7 months ago by vdelecroix

line 240-241 of src/setup.py the compile_time_env variable is set up (globally). It seems that it erases any specific configuration at an Extension level.

comment:13 Changed 7 months ago by dcoudert

outch... Well, either we are able to find a smart solution, or we could decide to support only cplex >= 12.8 (released Oct 2017). We already dropped support of older versions when the API introduced file cpxconst.h (#11958).

comment:14 Changed 6 months ago by git

  • Commit changed from 72e1ea0439875af0d45840767bd0cdf3b4761242 to 93ff118d5b71d43b0c0c4fc82f2bd331705070a9

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

93ff118trac #27790: another trial

comment:15 Changed 6 months ago by dcoudert

still not working :(

comment:16 Changed 6 months ago by asante

  • Cc Friedrich Wiemer added

comment:17 Changed 6 months ago by dcoudert

  • Milestone changed from sage-8.8 to sage-8.9

I'm more and more in favor of dropping support for cplex versions < 12.8. Indeed, we have an easy solution to support versions >= 12.8, but no simple, and more important working, solution for conditional compilation enabling to support < 12.8.

comment:18 Changed 4 months ago by dcoudert

  • Cc asante added; Friedrich Wiemer removed

another argument for dropping support of version < 12.8: the IBM academic initiative allows to get versions 12.8 and 12.9, and no prior version.

comment:19 Changed 2 months ago by dcoudert

  • Milestone changed from sage-8.9 to sage-9.0

We should either wait for #28382 or rebase this ticket on it as #28382 correct the types used in this backend.

comment:20 Changed 5 weeks ago by slabbe

I get the error below when I try to make sage 9.0.beta4 with the current branch:

[sagelib-9.0.beta4] [1/1] Cythonizing sage/numerical/backends/cplex_backend.pyx
[sagelib-9.0.beta4] /home/slabbe/GitBox/sage/local/lib/python2.7/site-packages/Cython/Compiler/Main.py:369: FutureWarning: Cython directive 'language_level' not set, using 2 for now (Py2). This will change in a later release! File: /home/slabbe/GitBox/sage/src/sage/numerical/backends/cplex_backend.pxd
[sagelib-9.0.beta4]   tree = Parsing.p_module(s, pxd, full_module_name)
[sagelib-9.0.beta4] Discovering Python/Cython source code....
[sagelib-9.0.beta4] Discovered Python/Cython sources, time: 0.02 seconds.
[sagelib-9.0.beta4] running build
[sagelib-9.0.beta4] Generating auto-generated sources
[sagelib-9.0.beta4] Building interpreters for fast_callable
[sagelib-9.0.beta4] running build_cython
[sagelib-9.0.beta4] Enabling Cython debugging support
[sagelib-9.0.beta4] Updating Cython code....
[sagelib-9.0.beta4] ************************************************************************
[sagelib-9.0.beta4] Traceback (most recent call last):
[sagelib-9.0.beta4]   File "setup.py", line 871, in <module>
[sagelib-9.0.beta4]     ext_modules = ext_modules)
[sagelib-9.0.beta4]   File "/home/slabbe/GitBox/sage/local/lib/python2.7/distutils/core.py", line 151, in setup
[sagelib-9.0.beta4]     dist.run_commands()
[sagelib-9.0.beta4]   File "/home/slabbe/GitBox/sage/local/lib/python2.7/distutils/dist.py", line 953, in run_commands
[sagelib-9.0.beta4]     self.run_command(cmd)
[sagelib-9.0.beta4]   File "/home/slabbe/GitBox/sage/local/lib/python2.7/distutils/dist.py", line 972, in run_command
[sagelib-9.0.beta4]     cmd_obj.run()
[sagelib-9.0.beta4]   File "setup.py", line 770, in run
[sagelib-9.0.beta4]     build.run(self)
[sagelib-9.0.beta4]   File "/home/slabbe/GitBox/sage/local/lib/python2.7/distutils/command/build.py", line 127, in run
[sagelib-9.0.beta4]     self.run_command(cmd_name)
[sagelib-9.0.beta4]   File "/home/slabbe/GitBox/sage/local/lib/python2.7/distutils/cmd.py", line 326, in run_command
[sagelib-9.0.beta4]     self.distribution.run_command(command)
[sagelib-9.0.beta4]   File "/home/slabbe/GitBox/sage/local/lib/python2.7/distutils/dist.py", line 972, in run_command
[sagelib-9.0.beta4]     cmd_obj.run()
[sagelib-9.0.beta4]   File "setup.py", line 323, in run
[sagelib-9.0.beta4]     cache=False,
[sagelib-9.0.beta4]   File "/home/slabbe/GitBox/sage/local/lib/python2.7/site-packages/Cython/Build/Dependencies.py", line 966, in cythonize
[sagelib-9.0.beta4]     aliases=aliases)
[sagelib-9.0.beta4]   File "/home/slabbe/GitBox/sage/local/lib/python2.7/site-packages/Cython/Build/Dependencies.py", line 808, in create_extension_list
[sagelib-9.0.beta4]     raise TypeError(msg)
[sagelib-9.0.beta4] TypeError: pattern is not of type str nor subclass of Extension (<class distutils.extension.Extension at 0x7fc827d040b8>) but of type <type 'list'> and class <type 'list'>
[sagelib-9.0.beta4] ************************************************************************
[sagelib-9.0.beta4] Error building the Sage library
[sagelib-9.0.beta4] ************************************************************************
[sagelib-9.0.beta4] Please email sage-devel (http://groups.google.com/group/sage-devel)
[sagelib-9.0.beta4] explaining the problem and including the relevant part of the log file
[sagelib-9.0.beta4]   /home/slabbe/GitBox/sage/logs/pkgs/sagelib-9.0.beta4.log
[sagelib-9.0.beta4] Describe your computer, operating system, etc.
[sagelib-9.0.beta4] ************************************************************************

comment:21 Changed 5 weeks ago by dcoudert

The current branch is not working. It's an unsuccessful trial to detect cplex version at compile time. I will push a new branch soon.

comment:22 Changed 5 weeks ago by dcoudert

  • Branch changed from public/numerical/27790_cplex_logfilename to public/linear_programming/27790_cplex
  • Commit changed from 93ff118d5b71d43b0c0c4fc82f2bd331705070a9 to abd00fe155472589cfb34c20047a853fc57a5d5b
  • Dependencies set to #28382

I pushed a new branch, built on top of 9.0.bet4 and #28382 (but not #28708, so corresponding doctests are still here).

The changes are in file cplex_backend.pxd

-     # sets the log stream file
-     int CPXsetlogfile (c_cpxlp * env, FILE * f)
+     # Set the log file
+     int CPXsetlogfilename(c_cpxenv * env, char * filename, char * mode)

and in file cplex_backend.pyx

-        cdef FILE *ff
         if name.lower() == "logfile":
             if value is None: # Return logfile name
                 return self._logfilename
             elif not value:   # Close current logfile and disable logs
-                check( CPXsetlogfile(self.env, NULL) )
+                check( CPXsetlogfilename(self.env, NULL, NULL) )
                 self._logfilename = ''
             else:             # Set log file to logfilename
-                ff = fopen(str_to_bytes(value), "a")
-                if not ff:
-                    raise ValueError("Unable to append file {}.".format(value))
-                check( CPXsetlogfile(self.env, ff) )
+                check( CPXsetlogfilename(self.env, str_to_bytes(value), "a") )
                 self._logfilename = value

The drawback of this branch is to loose compatibility with cplex versions prior to 12.8, but since it is more than 2 years old and that the current version is 12.9, this is certainly OK. May be we should add something in http://doc.sagemath.org/html/en/thematic_tutorials/linear_programming.html ?


New commits:

94026d1trac #28382: fix compilation warnings with cplex backends
f1b2279trac #27790: Merged 28382 with
abd00fetrac #27790: use CPXsetlogfilename instead of CPXsetlogfile

comment:23 Changed 4 weeks ago by git

  • Commit changed from abd00fe155472589cfb34c20047a853fc57a5d5b to 37dcc4d6f490d4e12c25470f1d3a53ae80876403

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

37dcc4dtrac #27790: update thematic tutorial on linear programming

comment:24 Changed 4 weeks ago by dcoudert

Some updates to the thematic tutorial: license file no longer needed and make it clear that we drop support fo versions < 12.8

comment:25 Changed 4 weeks ago by slabbe

  • Reviewers set to Sébastien Labbé
  • Status changed from needs_review to positive_review

Works for me with both 12.8 and 12.9.

I agree to drop support for versions < 12.8.

comment:26 Changed 4 weeks ago by dcoudert

Thank you so much for the great help. I will finally be able to use cplex 12.9 without patch :))

comment:27 Changed 4 weeks ago by slabbe

Great! I am sorry I took so long to come by and review your tickets.

comment:28 Changed 4 weeks ago by vbraun

  • Status changed from positive_review to needs_work

Merge conflict

comment:29 Changed 3 weeks ago by git

  • Commit changed from 37dcc4d6f490d4e12c25470f1d3a53ae80876403 to 0881f5b559cfabb53c8eac4d081bef893352cfa5

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

0881f5btrac #27790: fix merge conflict with 9.0.beta6

comment:30 Changed 3 weeks ago by dcoudert

  • Status changed from needs_work to needs_review

I fixed the merge conflicts with last beta.

comment:31 Changed 3 weeks ago by slabbe

  • Status changed from needs_review to positive_review

comment:32 Changed 3 weeks ago by dcoudert

Thanks !

comment:33 Changed 2 weeks ago by vbraun

  • Branch changed from public/linear_programming/27790_cplex to 0881f5b559cfabb53c8eac4d081bef893352cfa5
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.