-
Notifications
You must be signed in to change notification settings - Fork 1.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Enable MOSEK in wheel builds. #16927
Enable MOSEK in wheel builds. #16927
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The code looks good but we'll need to coordinate this merge with other simultaneous changes.
Reviewed 3 of 3 files at r1, all commit messages.
Reviewable status: 1 unresolved discussion, LGTM missing from assignee jwnimmer-tri(platform), needs at least two assigned reviewers, missing label for release notes (waiting on @mwoehlke-kitware)
tools/wheel/image/build-wheel.sh, line 31 at r1 (raw file):
cp -r -t /wheel/pydrake/lib \ /opt/drake/lib/libdrake*.so \ /opt/drake/lib/libmosek*.so*
According to drake/tools/workspace/mosek/repository.bzl
we need two shared libraries for MOSEK to run correctly ...
libcilkrts.so.5
libmosek64.so.{}.{}
... but here we are only copying one of them. I wonder if MOSEK actually runs correctly (once enabled with a license file)?
I can test that myself later this week, if you're unable to.
tools/wheel/image/build-wheel.sh, line 31 at r1 (raw file): Previously, jwnimmer-tri (Jeremy Nimmer) wrote…
I wouldn't know how to test that. I note that |
@drake-jenkins-bot linux-focal-unprovisioned-gcc-wheel-experimental-snopt-release please |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewable status: 2 unresolved discussions, LGTM missing from assignee jwnimmer-tri(platform), needs at least two assigned reviewers, missing label for release notes (waiting on @jwnimmer-tri and @mwoehlke-kitware)
a discussion (no related file):
Working
I'll plan to test this locally using a mosek license and a sample mathematical program, versus a newly-built wheel as of this PR.
tools/wheel/image/build-wheel.sh, line 31 at r1 (raw file):
Previously, mwoehlke-kitware (Matthew Woehlke) wrote…
I wouldn't know how to test that. I note that
auditwheel
didn't complain, which is why I didn't add that one. That also leads me to suspect that library is dynamically loaded (i.e.dlopen
), not linked?
The libmosek64.so.9.2
has:
0x0000000000000001 (NEEDED) Shared library: [libcilkrts.so.5]
The CI test is good, thanks. If that passes, then I'll pull this locally and investigate whether the library is already there, or why this is working.
tools/wheel/image/build-wheel.sh, line 31 at r1 (raw file): Previously, jwnimmer-tri (Jeremy Nimmer) wrote…
Interesting. Maybe it's an If you have a wheel already, Note that I have built this and run the "tests" (i.e. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewable status: 1 unresolved discussion, needs at least two assigned reviewers, missing label for release notes (waiting on @mwoehlke-kitware)
a discussion (no related file):
OK looks great in a venv, built locally:
jwnimmer@call-cps:~/jwnimmer-tri/drake$ env/bin/python
Python 3.8.10 (default, Mar 15 2022, 12:22:08)
[GCC 9.4.0] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> import pydrake
>>> from pydrake.solvers.mosek import MosekSolver
>>> dut = MosekSolver()
>>> dut.available()
True
>>> dut.enabled()
False
>>> import os
>>> os.environ['MOSEKLM_LICENSE_FILE'] = '/home/jwnimmer/Downloads/mosek.lic'
>>> dut.enabled()
True
>>> from pydrake.solvers import mathematicalprogram as mp
>>> prog = mp.MathematicalProgram()
>>> x = prog.NewContinuousVariables(2, "x")
>>> prog.AddLinearConstraint(x[0] >= 1)
<pydrake.solvers.mathematicalprogram.Binding[LinearConstraint] object at 0x7fa80a855e70>
>>> prog.AddLinearConstraint(x[1] >= 1)
<pydrake.solvers.mathematicalprogram.Binding[LinearConstraint] object at 0x7fa807aaa270>
>>> import numpy as np
>>> prog.AddQuadraticCost(np.eye(2), np.zeros(2), x)
<pydrake.solvers.mathematicalprogram.Binding[QuadraticCost] object at 0x7fa80a841c70>
>>> solver = MosekSolver()
>>> solver_options = mp.SolverOptions()
>>> solver_options.SetOption(mp.CommonSolverOption.kPrintToConsole, 1)
>>> assert solver.available()
>>> result = solver.Solve(prog, None, solver_options)
Problem
Name :
Objective sense : min
Type : QO (quadratic optimization problem)
Constraints : 0
Cones : 0
Scalar variables : 2
Matrix variables : 0
Integer variables : 0
Optimizer started.
Quadratic to conic reformulation started.
Quadratic to conic reformulation terminated. Time: 0.00
Presolve started.
Linear dependency checker started.
Linear dependency checker terminated.
Eliminator started.
Freed constraints in eliminator : 2
Eliminator terminated.
Eliminator started.
Freed constraints in eliminator : 0
Eliminator terminated.
Eliminator - tries : 2 time : 0.00
Lin. dep. - tries : 1 time : 0.00
Lin. dep. - number : 0
Presolve terminated. Time: 0.00
Problem
Name :
Objective sense : min
Type : QO (quadratic optimization problem)
Constraints : 0
Cones : 0
Scalar variables : 2
Matrix variables : 0
Integer variables : 0
Optimizer - threads : 24
Optimizer - solved problem : the primal
Optimizer - Constraints : 0
Optimizer - Cones : 1
Optimizer - Scalar variables : 3 conic : 3
Optimizer - Semi-definite variables: 0 scalarized : 0
Factor - setup time : 0.00 dense det. time : 0.00
Factor - ML order time : 0.00 GP order time : 0.00
Factor - nonzeros before factor : 0 after factor : 0
Factor - dense dim. : 0 flops : 0.00e+00
ITE PFEAS DFEAS GFEAS PRSTATUS POBJ DOBJ MU TIME
0 2.0e+00 0.0e+00 2.0e+00 0.00e+00 5.000000000e-01 0.000000000e+00 1.0e+00 0.01
1 4.0e-01 3.3e-16 2.5e-01 -3.33e-01 1.066166551e+00 8.653589735e-01 2.0e-01 0.01
2 1.6e-02 8.9e-16 1.3e-03 1.24e+00 1.005974311e+00 9.872686725e-01 8.2e-03 0.01
3 7.6e-07 6.0e-15 3.0e-10 1.01e+00 1.000000205e+00 9.999991174e-01 3.8e-07 0.01
4 1.1e-16 7.6e-10 1.0e-19 1.00e+00 1.000000000e+00 1.000000000e+00 2.2e-11 0.01
Optimizer terminated. Time: 0.01
Interior-point solution summary
Problem status : PRIMAL_AND_DUAL_FEASIBLE
Solution status : OPTIMAL
Primal. obj: 1.0000000001e+00 nrm: 1e+00 Viol. var: 0e+00
Dual. obj: 1.0000000002e+00 nrm: 1e+00 Viol. var: 0e+00
>>> assert result.is_success()
>>> print(result.GetSolution(x)) # 1, 1
[1. 1.]
tools/wheel/image/build-wheel.sh, line 31 at r1 (raw file):
Previously, mwoehlke-kitware (Matthew Woehlke) wrote…
Interesting. Maybe it's an
auditwheel
flake? (Or maybelibmosek64.so.9.2
has RPATHs that allowauditwheel
to find the library in/opt/drake
... I'd check if it wound up in my wheels, but the ones with MOSEK have been nuked. Maybe I'll do that tomorrow...)If you have a wheel already,
unzip -l drake<...>.whl | grep cilk
would be a quick check if the.so
made it in there...Note that I have built this and run the "tests" (i.e.
assert pydrake.all.SnoptSolver().available()
) locally.
I built a wheel locally. It looks like this:
pydrake/lib/libdrake.so
pydrake/lib/libdrake_lcm.so
pydrake/lib/libdrake_marker.so
pydrake/lib/libdrake_spdlog.so
pydrake/lib/libmosek64.so.9.2
drake.libs/libGLX-74c0d009.so.0.0.0
drake.libs/libGLdispatch-80f42924.so.0.0.0
drake.libs/libOpenGL-9502a6a8.so.0.0.0
drake.libs/libXt-f585519f.so.6.0.0
drake.libs/libcilkrts-49bbc578.so.5.0.0
drake.libs/libcrypt-ede83cdd.so.1.1.0
drake.libs/libgfortran-1488f340.so.5.0.0
drake.libs/libnlopt_cxx-f2a58e92.so.0.10.0
drake.libs/libquadmath-ab8ef433.so.0.0.0
Seems like libmosek is actually the one that's in the wrong place. I think we want mosek and cilk to be like GL, gfortran, etc -- bundled shared library dependencies from other vendors? So this file should be reverted here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewable status: 2 unresolved discussions, needs at least two assigned reviewers, missing label for release notes (waiting on @mwoehlke-kitware)
a discussion (no related file):
The code looks good but we'll need to coordinate this merge with other simultaneous changes.
(1) The website or needs needs to be updated (by me) to reflect our new MOSEK contract.
(2) The CI builds at https://drake-jenkins.csail.mit.edu/view/Wheel/ need to change names to say "MOSEK".
(3) We need to remind the next stable release after this merges to fix the website text that says "Drake's pip packages do not support the MOSEK™ nor Gurobi solvers.". Probably we should have a Draft PR open that fixes all of the MOSEK mentions (including binary and apt releases) all at once, sitting ready for the next release manager to merge at the appropriate time.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewable status: 2 unresolved discussions, needs at least two assigned reviewers (waiting on @mwoehlke-kitware)
a discussion (no related file):
Previously, jwnimmer-tri (Jeremy Nimmer) wrote…
The code looks good but we'll need to coordinate this merge with other simultaneous changes.
(1) The website or needs needs to be updated (by me) to reflect our new MOSEK contract.
(2) The CI builds at https://drake-jenkins.csail.mit.edu/view/Wheel/ need to change names to say "MOSEK".
(3) We need to remind the next stable release after this merges to fix the website text that says "Drake's pip packages do not support the MOSEK™ nor Gurobi solvers.". Probably we should have a Draft PR open that fixes all of the MOSEK mentions (including binary and apt releases) all at once, sitting ready for the next release manager to merge at the appropriate time.
Update on (3) -- the notes PR is up at #16939. Please submit the pip documentation changes there, either by pushing a commit to the branch, or posting a review comment with the requested text.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewable status: 1 unresolved discussion, needs at least two assigned reviewers (waiting on @mwoehlke-kitware)
a discussion (no related file):
Previously, jwnimmer-tri (Jeremy Nimmer) wrote…
Update on (3) -- the notes PR is up at #16939. Please submit the pip documentation changes there, either by pushing a commit to the branch, or posting a review comment with the requested text.
(1) is done. (2) is ready at https://github.com/RobotLocomotion/drake-jenkins-jobs/pull/3.
For (3), we don't need to block this PR, but please still work on it.
I'll mark this discussion as resolved.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can do +(status: single reviewer ok) here. Pending the shared library install location discussion below, we're ready to land this and the CI changes.
Reviewable status: 1 unresolved discussion (waiting on @mwoehlke-kitware)
tools/wheel/image/build-wheel.sh, line 31 at r1 (raw file): Previously, jwnimmer-tri (Jeremy Nimmer) wrote…
Okay. So, I suppose one option, which is sort of weird, would be to copy the MOSEK libraries to Thoughts? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewable status: 1 unresolved discussion (waiting on @mwoehlke-kitware)
tools/wheel/image/build-wheel.sh, line 31 at r1 (raw file):
I suppose one option, which is sort of weird, would be to copy the MOSEK libraries to /opt/drake-dependencies/lib.
That would be fine by me (with a comment).
Otherwise we would have to create some other location which holds only those libraries, and add that to LD_LIBRARY_PATH.
That's also fine (with a comment).
I think with either way, it's going to be a bit of an oddball case so best to just write a good explanation into the code; that will matter more than the choice of mechanism.
Please also check locally that the wheel ends up with the drake-install cilk library (and not the Ubuntu one).
tools/wheel/image/build-wheel.sh, line 31 at r1 (raw file):
Unfortunately, this may be difficult, as AFAICT |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewable status: 1 unresolved discussion (waiting on @mwoehlke-kitware)
tools/wheel/image/build-wheel.sh, line 31 at r1 (raw file):
Previously, mwoehlke-kitware (Matthew Woehlke) wrote…
Please also check locally that the wheel ends up with the drake-install cilk library (and not the Ubuntu one).
Unfortunately, this may be difficult, as AFAICT
auditwheel
rewrites the library, preventing simple checks (checksum, size) from being used to determine which library got included.
In that case, maybe nuke the hazardous libary (rm -f /usr/lib/x86.../libcilk.*
) prior to assembling the wheel?
tools/wheel/image/build-wheel.sh, line 31 at r1 (raw file): Previously, jwnimmer-tri (Jeremy Nimmer) wrote…
Oh, fun... I'm going to guess it's the right one, however:
It seems unlikely that |
0cfb11d
to
dc6f850
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewable status: 1 unresolved discussion (waiting on @jwnimmer-tri)
tools/wheel/image/build-wheel.sh, line 31 at r1 (raw file):
Previously, mwoehlke-kitware (Matthew Woehlke) wrote…
Oh, fun...
I'm going to guess it's the right one, however:
-rw-r--r-- 1 root root 122920 Mar 26 2020 /usr/lib/x86_64-linux-gnu/libcilkrts.so.5.0.0 -rwxr-xr-x 1 root root 244784 Apr 13 17:14 /opt/drake/lib/libcilkrts.so.5 -rwxr-xr-x 1 root root 260616 Apr 13 17:15 libcilkrts-931b5886.so.5
It seems unlikely that
auditwheel
managed to more than double the library's size?
Done.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewed 1 of 1 files at r2, all commit messages.
Reviewable status: complete! all discussions resolved, LGTM from assignee jwnimmer-tri(platform) (waiting on @mwoehlke-kitware)
* Enable MOSEK in wheel builds.
* Enable MOSEK in wheel builds.
Towards #16439.
The wheel now additionally contains:
(Maybe others; the above are just what matched
grep -i mosek
. Mostly to show/verify that license-related files are present.)+@jwnimmer-tri for feature review, please
This change is