Skip to content
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

Drake's build of Ipopt should not use Metis #4913

Closed
jwnimmer-tri opened this issue Jan 26, 2017 · 23 comments
Closed

Drake's build of Ipopt should not use Metis #4913

jwnimmer-tri opened this issue Jan 26, 2017 · 23 comments
Assignees
Labels
component: distribution Nightly binaries, monthly releases, docker, installation priority: backlog type: bug

Comments

@jwnimmer-tri
Copy link
Collaborator

Drake's build and use of Ipopt should not use Metis, due to licensing. See #4045 for the citation to the non-EPL parts of Ipopt.

I recommend re-doing https://github.com/RobotLocomotion/ipopt-mirror/blob/master/get_ipopt_source.sh to disable even putting Metis into our repository, which then I think the configure code within Ipopt will see its absence and invoke the work-around.

Alternatively, it looks like Ipopt has the ability to set configure switches to disable using Metis even when the source is present. That could possibly be an earlier first spiral.

@jwnimmer-tri jwnimmer-tri changed the title Drake's build of Ipopt should not use metis Drake's build of Ipopt should not use Metis Jan 26, 2017
@jefesaurus
Copy link
Contributor

http://glaros.dtc.umn.edu/gkhome/metis/metis/faq

"METIS as of 5.0.3 is being distributed under the Apache License Version 2.0."

I had to compile everything separately(metis 5, mumps, scotch if you want) instead of using ipopt's script by default, but it eventually worked without any patches.

@jwnimmer-tri
Copy link
Collaborator Author

Hey, that's neat. Perhaps @sammy-tri can decide what do to next here?

@jwnimmer-tri
Copy link
Collaborator Author

@sammy-tri If you can offer any guidance on the path forward, I could help either implement this or help @stonier delegate it. (Or redirect us to someone else who can make the decision?) As we move into a more mature release process, I would really like to remove software from our build that we don't have a license grant to use.

@sammy-tri
Copy link
Contributor

I don't have an opinion between figuring out how to integrate Metis 5 or disabling Metis. I've tried neither and don't know what the result would be.

I suppose I've been ignoring the priority:high tag (I hadn't actually noticed it until now). I can put this closer to the top of my todo list it it's urgently needed. Beyond the fact that I was the original author of get_ipopt_source.sh I don't think I'm any more qualified than any other developer if you do decide to delegate.

@jwnimmer-tri
Copy link
Collaborator Author

@sammy-tri Mostly I just wanted your input. It sounds like we should next seek a domain expert to decide if we need Metis or not.

I just pinged Russ about it. He suggests just to try turning it off and see if we notice. I will either do that myself or let @stonier steal it from me.

@jwnimmer-tri
Copy link
Collaborator Author

Step 1 will be to get get_ipopt_source.sh repeatable, by using a tagged release instead of the stable branch, along with some usability fixes so it works for PRs. Step 2 will be to blacklist METIS.

@jefesaurus
Copy link
Contributor

FWIW I just profiled some trajectory optimization using PORD the public domain default for MUMPS, and the linear solver was less than 6.5% of the CPU time. Presumably, PORD vs Metis is some relatively small fraction of that.

"initializeAutodiff" was 10x that (I'm may be doing something goofy there, too)

I didn't do much of a comparison between metis and pord, but I couldn't tell the difference.
2017-05-17-104341_1411x1085_scrot

@jwnimmer-tri
Copy link
Collaborator Author

@sammy-tri So Ubuntu has Ipopt already precompiled: https://launchpad.net/ubuntu/+source/coinor-ipopt/3.11.9-2.1. Homebrew appears to have it also. Do you recall why we were building it from source?

@sammy-tri
Copy link
Contributor

There are a lot of potential pitfalls, though I don't remember specifics. Concerns include:

  • Package availability on both Trusty and Xenial
  • Windows support was a thing when IPOPT was added (though we never added Windows support, and IIRC there are downloadable Windows binaries for IPOPT)
  • I almost certainly didn't think to check for homebrew packages
  • Possible MATLAB linkage problems involving the BLAS and LAPACK builds linked into IPOPT... This seems like a more probable gotcha since coinor-libipopt1v5 depends on dynamically linked BLAS and LAPACK and we statically compile them into our IPOPT... but I don't immediately recall testing this.

Sorry for not having more specifics.

@jwnimmer-tri
Copy link
Collaborator Author

jwnimmer-tri commented Jun 9, 2017

So given that ...

  • Ipopt is soon only going to come in via Bazel, and
  • the Bazel rule for it is just calling Ipopt's super-slow autotools-based build, and
  • the Ipopt autotools build doesn't use any of the Bazel CROSSTOOL settings (e.g., it's always using system GCC and doesn't obey debug vs release) so we don't even get good IDE and debuger support, and
  • it's mostly impossible to make it use CROSSTOOL settings unless we write BUILD files for it.

... I'm not sure that compiling Ipopt from source is buying us anything.

Furthermore, my attempt to remove Metis crashed and burned, I suspect there is going to be a bunch of futzing to either upgrade to newer Metis, or figure out what went wrong with what I was trying.

Given all of that, I wonder if perhaps the best approach is to precompile Ipopt (probably using some recipe from our ipopt-mirror) and then host that binary release of Ipopt for Drake as we do a few other things now, like vtk and drake-visualizer and etc. (In the long term, if we really think Ipopt is worth keeping, it would be awesome to write BUILD files; but that's going to be a lot of work.)

In any case, this is ending up as more work than anticipated, so I should probably punt it over to @stonier to organize. Possibly it would be a good project to receive Kitware's help. I think it's a blocker for doing a binary Drake release, since the licensing isn't really up to snuff until this is done -- and to my understanding, a binary Drake release with NLopt as the only solver would not be very useful.

@jwnimmer-tri jwnimmer-tri added unused team: kitware component: distribution Nightly binaries, monthly releases, docker, installation and removed unused team: manipulation labels Jun 9, 2017
@jwnimmer-tri jwnimmer-tri assigned stonier and unassigned jwnimmer-tri Jun 9, 2017
@RussTedrake
Copy link
Contributor

i seem to recall @hongkai-dai mentioning that ipopt was also distributed now by the ubuntu package managers...? but perhaps with metis .

@jwnimmer-tri
Copy link
Collaborator Author

@stonier Re-ping. I am uncomfortable having Drake provide binary releases to the public that include this landmine.

@jwnimmer-tri
Copy link
Collaborator Author

jwnimmer-tri commented Sep 13, 2017

Some thoughts on using the OS's build of Ipopt:

On Xenial, in Bazel, Ubuntu's Ipopt links and passes all tests.

On Xenial, in CMake + MATLAB, Ubuntu's Ipopt links libblas.so dynamically, causing MATLAB to segfault as Sam predicted.

On Trusty, if we wanted to use a deb package for Ipopt to match Xenial, we could conceivably rebuild Xenial's source package and self-host it. Alternatively, Debian Jessie is based on GCC 4.9 and has coinor-ipopt packages ready, so possibly we could install those directly, or rebuild them from source for Trusty.

Edit 1: Trusty already has debs, I just missed them the first time searching. Stay tuned...

Edit 2: They compile OK, but fail at runtime because https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=736842 and https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=763612 are unfixed in Trusty.

I haven't tried to test the homebrew packages yet. Maybe an OSX volunteer could do that:

--- a/WORKSPACE
+++ b/WORKSPACE
@@ -185,12 +185,9 @@
     build_file = "tools/fcl.BUILD",
 )
 
-github_archive(
+pkg_config_package(
     name = "ipopt",
-    repository = "RobotLocomotion/ipopt-mirror",
-    commit = "aecf5abd3913eebf1b99167c0edd4e65a6b414bc",
-    sha256 = "d88ea1b6b34c5678ef32ced22a6e9cb00f76a490f233d0b2d56270609eb94e3e",  # noqa
-    build_file = "tools/ipopt.BUILD",
+    modname = "ipopt",
 )
 
 github_archive(

Assuming homebrew can be made to work, and Trusty packages come together quickly, my suggested approach is to switch to debs & homebrew and defer solving the MATLAB linking problems until after the CMake transition is completed. That would let us get nightly / monthly releases out the door sooner rather than later, without having to delve into Ipopt's build system to fix licensing problems.

@soonho-tri
Copy link
Member

I haven't tried to test the homebrew packages yet. Maybe an OSX volunteer could do that:

I just tested it. ipopt provides a pre-compiled binary (a.k.a. 'bottle'), which is nice. It also has pkg-config file, ipopt.pc.

@RussTedrake
Copy link
Contributor

Fwiw, i consider ipopt + matlab to be a low priority corner case. Disabling it for the painful configurations would probably be better than having it block a release.

@jwnimmer-tri
Copy link
Collaborator Author

jwnimmer-tri commented Sep 13, 2017

So the bottom like is that Xenial and OSX system packages look good. The Trusty packages are old and have runtime bugs. We could either disable Ipopt (and require Snopt) on Trusty, or else respin new Trusty debs.

@jamiesnape
Copy link
Contributor

jamiesnape commented Sep 13, 2017

Our binary packages always have SNOPT support built in. If we are happy to begin to deprecate development on Trusty (as opposed to usage), I would say just ignore IPOPT on that platform.

@jwnimmer-tri jwnimmer-tri self-assigned this Sep 13, 2017
@jwnimmer-tri
Copy link
Collaborator Author

jwnimmer-tri commented Sep 13, 2017

Taking the Vivid source package and pushing it through a Trusty pbuilder yields debs that pass all of our Bazel unit tests on Trusty. So creating them is not hard, it's just a question of whether we want to host them indefinitely. Assuming we want to host -doc and -dbg (not just lib and -dev), the total size is 9.0MB.

@jamiesnape
Copy link
Contributor

Personally, I would rather we not maintain more packages, especially ones that may conflict with the default IPOPT on Trusty.

@jwnimmer-tri jwnimmer-tri removed their assignment Sep 13, 2017
@jwnimmer-tri
Copy link
Collaborator Author

FWIW, I seriously doubt anyone is using the default IPOPT in Trusty, since it appears to abort your program anytime you try to invoke it, due to https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=736842, so I am not really worried about package conflicts.

In any case, I am not tied to any particular solution. Possibly deprecating Trusty for ongoing development is the right answer. Daniel will have to decide the path forward in support of a binary release.

Maybe we should try switching Xenial and OSX to use the system package in the meantime, as experiment to see if it works as well as we think?

@jwnimmer-tri jwnimmer-tri self-assigned this Sep 19, 2017
@jwnimmer-tri
Copy link
Collaborator Author

jwnimmer-tri commented Sep 19, 2017

From discussion with Daniel: focus on solving this for Mac and Xenial, most likely by using the system version of IPOPT. It is likely that Trusty support will disappear from master in short enough timeframe that we don't have to worry about it.

I've updated my #7027 to use system IPOPT on Apple and Xenial, but still compile from source on Trusty. I think merging that to master in the meantime would be a good way to shake out problems early.

@jwnimmer-tri
Copy link
Collaborator Author

Reducing priority, because since 7027 merged this is only relevant for Trusty, and we are not going to releases on Trusty. We can close this issue after Trusty is retired.

@jwnimmer-tri
Copy link
Collaborator Author

To circle back here -- METIS as of 5.0.3 is being distributed under Apache-2.0, so we could start using it again if we wanted to.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: distribution Nightly binaries, monthly releases, docker, installation priority: backlog type: bug
Projects
None yet
Development

No branches or pull requests

7 participants