Fix a Bug in GCS When Passing in a Trivially-Infeasible Upper Bound

[cohnt]

There was a slight logic error, where we were checking the lower bound instead of the upper bound. The current test cases didn't cover it because they didn't make sure that code path was tested. This PR adds test cases to reveal the bug, and implements the fix.

---

This change is 

[cohnt]

+@RussTedrake mind taking this one? Hopefully it should be pretty quick, but I think it ought to merge before #22091.

[RussTedrake]

(i had already reviewed it as a part of #22091, but I like that you've separated it out)

Reviewed 2 of 2 files at r1, all commit messages.
Reviewable status: needs at least two assigned reviewers, commits need curation (https://drake.mit.edu/reviewable.html#curated-commits) (waiting on @cohnt)

[RussTedrake]

+@rpoyner-tri for platform review, please.

Reviewable status: LGTM missing from assignee rpoyner-tri(platform), commits need curation (https://drake.mit.edu/reviewable.html#curated-commits) (waiting on @cohnt)

[rpoyner-tri]

Reviewed 2 of 2 files at r1, all commit messages.
Reviewable status: 1 unresolved discussion, commits need curation (https://drake.mit.edu/reviewable.html#curated-commits) (waiting on @cohnt)

---

geometry/optimization/test/graph_of_convex_sets_test.cc line 1773 at r1 (raw file):

  // trivially-infeasible constraint, so solving should throw an error.
  DRAKE_EXPECT_THROWS_MESSAGE(
      g_.SolveShortestPath(*source_, *target_, options_), ".*inf.*");

minor: this test clause should probably match on a bit more of the expected error text -- we have other throws in the same module with either "inf" for infinity or "infeasible".

[cohnt]

Reviewable status: commits need curation (https://drake.mit.edu/reviewable.html#curated-commits) (waiting on @cohnt)

---

geometry/optimization/test/graph_of_convex_sets_test.cc line 1773 at r1 (raw file):

Previously, rpoyner-tri (Rick Poyner (rico)) wrote…

minor: this test clause should probably match on a bit more of the expected error text -- we have other throws in the same module with either "inf" for infinity or "infeasible".

Done.

[rpoyner-tri]

Reviewed 2 of 2 files at r3, all commit messages.
Reviewable status: 1 unresolved discussion, commits need curation (https://drake.mit.edu/reviewable.html#curated-commits) (waiting on @cohnt)

---

geometry/optimization/test/graph_of_convex_sets_test.cc line 1819 at r3 (raw file):

  DRAKE_EXPECT_THROWS_MESSAGE(
      g_.SolveShortestPath(*source_, *target_, options_),
      ".*trivially-infeasible.*");

nit this is better; thanks. My last question is this: there are two error mesages in the implementation that match this pattern. One is for lower-bound failure, the other for upper-bound failure. Given that we are here fixing a lower-upper bound confusion already, is it worth having the test be able to sense which message actually got thrown?

[cohnt]

Reviewable status: 1 unresolved discussion, commits need curation (https://drake.mit.edu/reviewable.html#curated-commits) (waiting on @cohnt)

---

geometry/optimization/test/graph_of_convex_sets_test.cc line 1819 at r3 (raw file):

Previously, rpoyner-tri (Rick Poyner (rico)) wrote…

nit this is better; thanks. My last question is this: there are two error mesages in the implementation that match this pattern. One is for lower-bound failure, the other for upper-bound failure. Given that we are here fixing a lower-upper bound confusion already, is it worth having the test be able to sense which message actually got thrown?

I made it also match x >= +inf or x <= -inf, so we can tell which case it's hitting.

[rpoyner-tri]

Reviewed 2 of 2 files at r5, all commit messages.
Reviewable status: commits need curation (https://drake.mit.edu/reviewable.html#curated-commits) (waiting on @cohnt)