Support Gurobi 9.5.x

[mwoehlke-kitware]

Remove logic that limits Gurobi to 9.5.1, in order to support 9.5.x at any patch version. Update documentation accordingly.

Towards #14157.

---

This change is 

[mwoehlke-kitware]

+@jwnimmer-tri for feature review, please.

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 @jwnimmer-tri)

---

doc/_pages/bazel.md line 183 at r1 (raw file):

2. Set up your Gurobi license file in accordance with Gurobi documentation.
3. ``export GRB_LICENSE_FILE=/path/to/gurobi.lic``.
4. Download ``gurobi9.5.1_linux64.tar.gz``. You may need to manually edit the URL to get the correct version.

BTW, we still mention "9.5.1" here. I didn't have any good ideas how to reword this to say "any 9.5.x version" without making it more obtuse. Suggestions? (Is it okay as-is?)

[jwnimmer-tri]

feature, modulo open discussions.

+(release notes: feature)

Reviewed 6 of 6 files at r1, all commit messages.
Reviewable status: 4 unresolved discussions, needs at least two assigned reviewers (waiting on @mwoehlke-kitware)

a discussion (no related file):
The file drake/cmake/modules/FindGurobi.cmake looks like it should have been updated as well with revised logic.

I think you need to prepare a suite of CI runs that only have 9.5.2 installed, and prove that this PR passes all of those.

---

---

doc/_pages/bazel.md line 183 at r1 (raw file):

Previously, mwoehlke-kitware (Matthew Woehlke) wrote…

BTW, we still mention "9.5.1" here. I didn't have any good ideas how to reword this to say "any 9.5.x version" without making it more obtuse. Suggestions? (Is it okay as-is?)

Clarity for users is the most important; we can sacrifice compactness if we need to.

I think we should:

(1) explicitly say in this section of the documentation that Drake supports any 9.5.x patch version, and
(2) explicitly say that if the user has a different patch version that shown here, to just update all of the paths in these instructions to align with their patch version, and
(3) switch our canonical example here to be 9.5.2, since that's what users are currently being offered by the Gurobi Download Center (also for macOS on line 192 below).

---

tools/workspace/gurobi/package-ubuntu.BUILD.bazel.in line 28 at r1 (raw file):

]) or [":error-message.h"]

# In the Gurobi package, libgurobi95.so is a symlink to libgurobi.so.9.0.0.

typo

Ditto on the next line as well.

Suggestion:

9.5.1

---

tools/workspace/gurobi/repository.bzl line 17 at r1 (raw file):

    if os_result.is_macos:
        # Gurobi must be installed into its standard location.
        gurobi_home = "/Library/gurobi951/macos_universal2"

On first read, this does not seem to support 9.5.2 on macOS, since it hard-codes gurobi951 with no option to change it.

My guess at the best answer would be to glob for gurobi95* to find all 9.5.x versions, and then choose the one with the largest patch version.

When finding the largest patch version, I think it's fine to ignore the possibility of two-digit patch versions (>= 10). Gurobi's patch versions typically are <= 3 in practice over the years.

If we go that route, then we should play the same game on Ubuntu where anything in /opt/gurobi95*/linux64 is acceptable and we choose the newest. (The GUROBI_HOME would still take precedence.)

[mwoehlke-kitware]

Reviewable status: 3 unresolved discussions, needs at least two assigned reviewers (waiting on @jwnimmer-tri)

a discussion (no related file):

Previously, jwnimmer-tri (Jeremy Nimmer) wrote…

The file drake/cmake/modules/FindGurobi.cmake looks like it should have been updated as well with revised logic.

I think you need to prepare a suite of CI runs that only have 9.5.2 installed, and prove that this PR passes all of those.

FindGurobi.cmake has been updated. Some of this needs to run on CI, as I don't have a local install in /opt.

---

---

tools/workspace/gurobi/package-ubuntu.BUILD.bazel.in line 28 at r1 (raw file):

Previously, jwnimmer-tri (Jeremy Nimmer) wrote…

typo

Ditto on the next line as well.

Done.

---

tools/workspace/gurobi/repository.bzl line 17 at r1 (raw file):

Previously, jwnimmer-tri (Jeremy Nimmer) wrote…

On first read, this does not seem to support 9.5.2 on macOS, since it hard-codes gurobi951 with no option to change it.

My guess at the best answer would be to glob for gurobi95* to find all 9.5.x versions, and then choose the one with the largest patch version.

When finding the largest patch version, I think it's fine to ignore the possibility of two-digit patch versions (>= 10). Gurobi's patch versions typically are <= 3 in practice over the years.

If we go that route, then we should play the same game on Ubuntu where anything in /opt/gurobi95*/linux64 is acceptable and we choose the newest. (The GUROBI_HOME would still take precedence.)

Done.

[jwnimmer-tri]

Reviewed 4 of 4 files at r2, all commit messages.
Reviewable status: 2 unresolved discussions, needs at least two assigned reviewers (waiting on @mwoehlke-kitware)

---

tools/workspace/gurobi/repository.bzl line 17 at r2 (raw file):

    if best_dir == None:
        fail("Cannot find gurobi")

When the directory does not exist, we need to defer the error to build-time instead of load-time. Therefore, we cannot call fail() here. Instead, we should return a dummy path, e.g., prefix + "xxx" + postfix.

[mwoehlke-kitware]

Yup, wasn't sure if this should be feature or fix. Thanks.

Reviewable status: 2 unresolved discussions, needs at least two assigned reviewers (waiting on @jwnimmer-tri)

---

tools/workspace/gurobi/repository.bzl line 17 at r2 (raw file):

Previously, jwnimmer-tri (Jeremy Nimmer) wrote…

When the directory does not exist, we need to defer the error to build-time instead of load-time. Therefore, we cannot call fail() here. Instead, we should return a dummy path, e.g., prefix + "xxx" + postfix.

That is going to produce a somewhat confusing error message, "Gurobi 9.5 is not installed at /opt/gurobixxx...". Is that acceptable? Do you prefer literal xxx, or maybe /opt/gurboi-notfound, or /opt/gurobi<VER>? Or should the build-time error be changed to remove mention of the non-existing path?)

[jwnimmer-tri]

Reviewed 1 of 1 files at r3, all commit messages.
Reviewable status: 2 unresolved discussions, needs at least two assigned reviewers (waiting on @mwoehlke-kitware)

a discussion (no related file):

Previously, mwoehlke-kitware (Matthew Woehlke) wrote…

FindGurobi.cmake has been updated. Some of this needs to run on CI, as I don't have a local install in /opt.

That's fine. I just need to see the CI evidence before we close out this discussion thread.

---

---

tools/workspace/gurobi/repository.bzl line 17 at r2 (raw file):

Previously, mwoehlke-kitware (Matthew Woehlke) wrote…

That is going to produce a somewhat confusing error message, "Gurobi 9.5 is not installed at /opt/gurobixxx...". Is that acceptable? Do you prefer literal xxx, or maybe /opt/gurboi-notfound, or /opt/gurobi<VER>? Or should the build-time error be changed to remove mention of the non-existing path?)

I think the way here currently ("notfound") is fine.

---

tools/workspace/gurobi/repository.bzl line 10 at r3 (raw file):

    best_dir = None
    best_version = None
    for d in native.glob(prefix + "*" + postfix):

There is a typo here failing in CI. The argument to glob is a sequence of pattern strings, not a string.

Suggestion:

[prefix + "*" + postfix]

[mwoehlke-kitware]

Reviewable status: 1 unresolved discussion, needs at least two assigned reviewers (waiting on @jwnimmer-tri)

---

tools/workspace/gurobi/repository.bzl line 10 at r3 (raw file):

Previously, jwnimmer-tri (Jeremy Nimmer) wrote…

There is a typo here failing in CI. The argument to glob is a sequence of pattern strings, not a string.

Done.

[jwnimmer-tri]

Reviewed 1 of 1 files at r4, all commit messages.
Reviewable status: 1 unresolved discussion, needs at least two assigned reviewers (waiting on @mwoehlke-kitware)

[jwnimmer-tri]

Reviewed 1 of 1 files at r5, all commit messages.
Reviewable status: 1 unresolved discussion, needs at least two assigned reviewers (waiting on @mwoehlke-kitware)

[jwnimmer-tri]

CI is still unhappy here. Even without a copy of Gurobi available locally, you should still be able to touch some empty files as a mock-up of what the Gurobi binaries look like, in order to exercise & debug the Bazel logic (even though the compiler would still eventually be unhappy later on).

[mwoehlke-kitware]

CI says "The native module can be accessed only from a BUILD thread". So "my guess at the best answer would be to glob for gurobi95* to find all 9.5.x versions" doesn't seem workable. I took a brief look at trying to delay globbing until the BUILD, and it isn't obvious how to do that, given the need for BUILD to be declarative. Any other ideas?

[jwnimmer-tri]

Perhaps pass repo_ctx into _find_latest and then do something like this:

def _find_latest(repo_ctx, prefix, postfix):
    for patch_version in range(9, -1, -1):
        candidate = repo_ctx.path(prefix + patch_version + postfix)
        if candidate.exists():
            return candidate
    return path(prefix + "-notfound" + postfix)

[jwnimmer-tri]

Oh duh. We have https://bazel.build/rules/lib/path#readdir to list a directory. We could call that and then filter the results ourselves to find the subdirectories that match our required pattern.

[jwnimmer-tri]

+@SeanCurtis-TRI for platform review per schedule, please.

Reviewed 1 of 1 files at r6, all commit messages.
Reviewable status: 1 unresolved discussion, LGTM missing from assignee SeanCurtis-TRI(platform) (waiting on @mwoehlke-kitware and @SeanCurtis-TRI)

a discussion (no related file):

Previously, jwnimmer-tri (Jeremy Nimmer) wrote…

That's fine. I just need to see the CI evidence before we close out this discussion thread.

So the evidence we have so far here is just that the new glob-like logic succeeds when run against our Drake CI with Gurobi 9.5.1 installed. I'm OK merging this PR on the basis of that evidence; it feels pretty convincing.

However, I've edited the PR overview to say Towards #14157 now instead of closing the issue. Once this code merges, we can re-assign the issue to the CI maintainers and ask them to upgrade CI to have only Gurobi 9.5.2 installed. Once that change passes, that will prove that the issue is fully resolved. That seems easier that trying to test this PR against a canary image of 9.5.2.

---

---

tools/workspace/gurobi/repository.bzl line 7 at r6 (raw file):

load("@drake//tools/workspace:os.bzl", "determine_os")

def _find_latest(repo_ctx, path, prefix, subdir):

nit These arguments are all similar enough that I think we need a docstring here to help clarify what's going on. Something like:

path: The parent directory where Gurobi would typically be installed, e.g., /opt or /Library.

prefix: A directory name pattern for Drake's supported major-minor version of Gurobi. We will search path to find a installed copy of Gurobi.

subdir: The platform-specific subdirectory within the Gurobi release, e.g., linux64.

[SeanCurtis-TRI]

modulo the docstring requested below.

Reviewed 3 of 6 files at r1, 2 of 4 files at r2, 1 of 1 files at r5, 1 of 1 files at r6, all commit messages.
Reviewable status: 2 unresolved discussions (waiting on @jwnimmer-tri and @mwoehlke-kitware)

---

-- commits line 9 at r6:
BTW I have the vague sense that we should prefer leaving github data out of commit messages. THey're great in the github PR description but less so in commits. However, if @jwnimmer-tri didn't flag this, then I presume that vague sense I have has gone awry. Jeremy, have I misapplied a principle here?

---

tools/workspace/gurobi/repository.bzl line 7 at r6 (raw file):

Previously, jwnimmer-tri (Jeremy Nimmer) wrote…

nit These arguments are all similar enough that I think we need a docstring here to help clarify what's going on. Something like:

path: The parent directory where Gurobi would typically be installed, e.g., /opt or /Library.

prefix: A directory name pattern for Drake's supported major-minor version of Gurobi. We will search path to find a installed copy of Gurobi.

subdir: The platform-specific subdirectory within the Gurobi release, e.g., linux64.

Agreed.

It seems to me that the gurobi release will be in path/prefix/*/subdir or some such thing. (I'm not sure what the appropriate syntax to indicate zero or more sub-directories between prefix and subdir.) But that kind of visible concatenation might also contribute to understanding the relationship.

(I may also be fuzzy on whether we expect prefix to be a child directory of path or merely a descendant. I've assumed child here.)

[jwnimmer-tri]

Reviewable status: 1 unresolved discussion (waiting on @mwoehlke-kitware)

---

-- commits line 9 at r6:

Previously, SeanCurtis-TRI (Sean Curtis) wrote…

BTW I have the vague sense that we should prefer leaving github data out of commit messages. THey're great in the github PR description but less so in commits. However, if @jwnimmer-tri didn't flag this, then I presume that vague sense I have has gone awry. Jeremy, have I misapplied a principle here?

We don't have an official policy.

As an educational experience, sometimes I do remind co-workers that putting an issue number into a commit message turns into an incredible amount of issue-timeline spam:

However, at the moment it's at the author's discretion how they handle this. I avoid it myself, but it's not a policy.

[SeanCurtis-TRI]

Reviewable status: 1 unresolved discussion (waiting on @jwnimmer-tri)

---

-- commits line 9 at r6:

Previously, jwnimmer-tri (Jeremy Nimmer) wrote…

We don't have an official policy.

As an educational experience, sometimes I do remind co-workers that putting and issue number into a commit message turns into an incredible amount of issue-timeline spam:

However, at the moment it's at the author's discretion how they handle this. I avoid it myself, but it's not a policy.

Thanks

[mwoehlke-kitware]

Reviewable status: complete! all discussions resolved, LGTM from assignees jwnimmer-tri(platform),SeanCurtis-TRI(platform) (waiting on @jwnimmer-tri and @SeanCurtis-TRI)

---

tools/workspace/gurobi/repository.bzl line 7 at r6 (raw file):

Previously, SeanCurtis-TRI (Sean Curtis) wrote…

Agreed.

It seems to me that the gurobi release will be in path/prefix/*/subdir or some such thing. (I'm not sure what the appropriate syntax to indicate zero or more sub-directories between prefix and subdir.) But that kind of visible concatenation might also contribute to understanding the relationship.

(I may also be fuzzy on whether we expect prefix to be a child directory of path or merely a descendant. I've assumed child here.)

It's equivalent to globbing for f'{path}/{prefix}*/{subdir}' 🙂. I added a note to that effect.

[jwnimmer-tri]

Reviewed 1 of 1 files at r7, all commit messages.
Reviewable status: complete! all discussions resolved, LGTM from assignees jwnimmer-tri(platform),SeanCurtis-TRI(platform) (waiting on @mwoehlke-kitware)

[SeanCurtis-TRI]

Reviewed 1 of 1 files at r7, all commit messages.
Reviewable status: complete! all discussions resolved, LGTM from assignees jwnimmer-tri(platform),SeanCurtis-TRI(platform) (waiting on @mwoehlke-kitware)

[jwnimmer-tri]

Oops. All three of us forgot to test on macOS. Hotfixed in #18563.