sage_numerical_backends_coin fix #40964
Conversation
|
at the moment the upstream package file is just a temporary file, but I'll make a proper one. I propose stop linking the version of this package and the Sage version. |
|
Documentation preview for this PR (built with commit 360d485; changes) is ready! 🎉 |
|
What's the right way to test this PR ? I tried this and it fails |
|
no, just think of it as an optional spkg. Look up the configure option to use to enable it, run ./configure with this option, then make, etc. |
|
I can't compile cbc 2.9.4.p0. homebrew version is 2.10.12. |
does it work with with the Homebrew version?
either your LDFLAGS are unclean, or cbc's configure finds these cbc 2.10 has more dependencies, so bumping up the version isn't too easy. |
|
configure with option I tried with option |
|
OK, I see - cbc depends on openblas, but Homebrew's openblas is incompatible with Sage, as it uses libgomp.dylib, and cannot be used with clang (not without extra hacks). Namely: (and Apple clang doesn't grok |
dimpase
force-pushed
the
numerical_backends_coin-fix
branch
from
2baba14 to
03caa14
Compare
|
with the latest beta, 10.8.beta7, this much simplified branch works for me on Linux |
|
the next step is to update cbc spkg. I am trying the latest version cbc 2.10.12. It has a number of new dependencies, of which dylp is broken and semi-obsolete, so it has to be configured with I am getting errors in MILP code, e.g. in and very similar elsewhere - which seem to indicate that Cbc is multithreaded now, but Sage is not using this option correctly (which is not a surprise). I'll try do disable threads and see how it goes, but that's certainly not the optimal way to use this software. |
|
unfortunately, this leads to an already known rabbit hole, leading to cvxpy spkg, which is in a bad shape, for reasons not really of our only making - see e.g. osqp/qdldl-python#71 |
That is sufficient to revive the release github workflow. Done in #41128. |
dimpase
force-pushed
the
numerical_backends_coin-fix
branch
from
03caa14 to
360d485
Compare
|
@dcoudert - this fixes everything related to cbc - provided you have an external cbc accepted by Sage's configure. We re-incorporate We should do it conditionally - depending on presence of E.g. you can build one using It should also work on Conda. To fix the threading issue I for the time being commented out the |
@tobiasdiez - is |
|
@tobiasdiez - how does one make sure that |
| override_options: ['cython_language=cpp'], | ||
| include_directories: [inc_cpython, inc_rings], | ||
| dependencies: [py_dep, cysignals, gmp], | ||
| dependencies: [py_dep, cysignals, gmp, cbc], |
There was a problem hiding this comment.
Please extract a deps = [py_dep, cysignals, gmp] variable, and then add cbc conditionally only for the coin_backend (see similar constructions in other meson files)
There was a problem hiding this comment.
coin_backend doesn't need gmp, in fact. Should it just have dependencies: [py_dep, cysignals, cbc], ?
There was a problem hiding this comment.
in that case add gmp conditionally only for the other backend. In dummy code:
deps = [py_dep, cysignals]
if coin:
deps += cbc
else
deps += gmp
| sage: COIN()._is_present() # optional - sage_numerical_backends_coin | ||
| FeatureTestResult('sage_numerical_backends_coin', True) | ||
| """ | ||
| JoinFeature.__init__(self, 'sage_numerical_backends_coin', |
There was a problem hiding this comment.
I think it also makes sense to rename the feature now to cbc (if it's not used in too many places)
There was a problem hiding this comment.
right, I didn't rename the feature yet just to make the diff more readable. Will do
yes, that's the purpose of |
dimpase
force-pushed
the
numerical_backends_coin-fix
branch
from
3732a4b to
0758045
Compare
|
@mantepse - have you ever tried |
Yes, they used to work at least with scip - but making them work involved quite a bit of work. |
|
could you post the failures? |
|
One thing the bijectionist uses is to add constraints temporarily. |
one of them is related to the last commit here. * changed the test for |
|
Do I understand correctly that the length of the list is 217 with cbc? |
yes. and here are the other failures on my machine (the latest cbc, 2.10.12): |
dimpase
changed the title
|
@tobiasdiez - what's the role of @dimpase I got an action error with something about updated meson file. The error output showed the diff of the changed |
|
regarding Cbc is easy to install using their preferred way (and many distros provide it). |
|
I ran the tests with SCIP, all but two tests, including the long ones, pass. The two which don't pass are These are differences which are mathematically OK. Of course, the tests could possibly be improved. |
|
these tests are easy to improve - just collect the known outcomes for different solvers in a list, and test that the result is in the list. |
|
I installed cbc using coinbrew as explained above. and then make fails trying to compile cbc 2.9.4.p0 |
|
you need to put the location of coinbrew-installed cbc into |
I tried with To ensure that it's before homebrew once, I have to do the export after bootstrap and source .homebrew... and before configure.... |
|
one should run This should return |
|
@dcoudert - you need |
|
@dcoudert is your coinbrew cbc actually dependent on openblas? Same question for the homebrew one (it's declared to be dependent on openblas, but is it actually picked up during the build?) |
|
no dependency on openblas when using the coinbrew version, but there is one with the hombre version. Content of the file cbc.pc when building with coinbrew and the output of Now for the homebrew version: and here we get a dependency on openblas |
|
coinbrew version depends on Accelerate (Apple's blas/lapack). you may try my experimental branch on #40397 (sagemath with Accelerate instead of openblas on macOS) - I need to update it first |
|
I tried to combine with #40397 but it's the same result, even when ensuring that homebrew openblas is not in |
I just updated the instructions for #40397 - there is no special step needed any more, I forgot to do this. But this should not affect the result (adding a non-existent path to PKG_CONFIG_PATH is (probably) harmless). What do you see if you run (after ? And please attach |
5 participants
re-add
sage_numerical_backends_coininto sagelib📝 Checklist
⌛ Dependencies