Opened 3 years ago
Closed 2 years 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, GitHub, GitLab) | 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 3 years ago by
- Branch set to public/numerical/27790_cplex_logfilename
- Commit set to 62a189900d82cb2a35ae369872950b9a01365083
- Status changed from new to needs_review
comment:2 follow-up: ↓ 4 Changed 3 years ago by
- merge failure
- 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 3 years ago by
- Commit changed from 62a189900d82cb2a35ae369872950b9a01365083 to 61e518719f22fe5d1456d1a2883df9405f59c8aa
Branch pushed to git repo; I updated commit sha1. New commits:
61e5187 | trac #27790: fix merge conflict with 8.8.beta6
|
comment:4 in reply to: ↑ 2 ; follow-up: ↓ 5 Changed 3 years ago by
Replying to vdelecroix:
- merge failure
fixed
- 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 3 years ago by
Replying to dcoudert:
Replying to vdelecroix:
- 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
fromcpxconst.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 3 years ago by
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 3 years ago by
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 3 years ago by
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 3 years ago by
try compile_time_env
instead of define_macros
(see cython thread)
comment:10 Changed 3 years ago by
- Commit changed from 61e518719f22fe5d1456d1a2883df9405f59c8aa to 72e1ea0439875af0d45840767bd0cdf3b4761242
Branch pushed to git repo; I updated commit sha1. New commits:
72e1ea0 | trac #27790: try to use compile time parameter
|
comment:11 Changed 3 years ago by
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 3 years ago by
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 3 years ago by
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 3 years ago by
- Commit changed from 72e1ea0439875af0d45840767bd0cdf3b4761242 to 93ff118d5b71d43b0c0c4fc82f2bd331705070a9
Branch pushed to git repo; I updated commit sha1. New commits:
93ff118 | trac #27790: another trial
|
comment:15 Changed 3 years ago by
still not working :(
comment:16 Changed 3 years ago by
- Cc Friedrich Wiemer added
comment:17 Changed 3 years ago by
- 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 3 years ago by
- 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 3 years ago by
- Milestone changed from sage-8.9 to sage-9.0
comment:20 Changed 3 years ago by
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 3 years ago by
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 3 years ago by
- 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:
94026d1 | trac #28382: fix compilation warnings with cplex backends
|
f1b2279 | trac #27790: Merged 28382 with
|
abd00fe | trac #27790: use CPXsetlogfilename instead of CPXsetlogfile
|
comment:23 Changed 3 years ago by
- Commit changed from abd00fe155472589cfb34c20047a853fc57a5d5b to 37dcc4d6f490d4e12c25470f1d3a53ae80876403
Branch pushed to git repo; I updated commit sha1. New commits:
37dcc4d | trac #27790: update thematic tutorial on linear programming
|
comment:24 Changed 3 years ago by
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 3 years ago by
- 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 3 years ago by
Thank you so much for the great help. I will finally be able to use cplex 12.9 without patch :))
comment:27 Changed 3 years ago by
Great! I am sorry I took so long to come by and review your tickets.
comment:29 Changed 2 years ago by
- Commit changed from 37dcc4d6f490d4e12c25470f1d3a53ae80876403 to 0881f5b559cfabb53c8eac4d081bef893352cfa5
Branch pushed to git repo; I updated commit sha1. New commits:
0881f5b | trac #27790: fix merge conflict with 9.0.beta6
|
comment:30 Changed 2 years ago by
- Status changed from needs_work to needs_review
I fixed the merge conflicts with last beta.
comment:31 Changed 2 years ago by
- Status changed from needs_review to positive_review
comment:32 Changed 2 years ago by
Thanks !
comment:33 Changed 2 years ago by
- Branch changed from public/linear_programming/27790_cplex to 0881f5b559cfabb53c8eac4d081bef893352cfa5
- Resolution set to fixed
- Status changed from positive_review to closed
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 andCPXsetlogfilename
if version >= 12.8. Then, we can useCPXversionnumber
to adjust usage.New commits:
trac #27790: use CPXsetlogfilename