Add platform-specific test support (and fix testCMCGait10dof18musc.cpp)

[nickbianco]

Brief summary of changes

I'm tired of this test failing on the CI, so I'm finally attempting to implement a fix, or at least a reasonable patch for it. I wanted to avoid the crudest fix which would be to increase the test tolerance, so instead for my first attempt I've updated the std file.

This test is only failing on Mac, possibly due to small differences in how the IPOPT dependencies are built on Mac versus Windows/Linux. The "true" fix might be far more involved, but I'd prefer this PR to be a reasonable compromise between test coverage and stability in the CI.

Note: I updated the std file based on a solution I created on my Windows desktop, which may not work. In fact, this fix might not work at all since the Mac failures have been spurious.

Testing I've completed

Unless anyone has a better idea, I was planning to re-run the Mac CI a bunch of times and see what happens.

Looking for feedback on...

CHANGELOG.md (choose one)

• no need to update because...CI update.

---

This change is 

[nickbianco]

@aymanhab @carmichaelong @tkuchida thoughts?

[tkuchida]

Perhaps it isn't a big issue as long as the change wasn't dramatic and the test still passes on Windows. However, another option could be to create a std file specifically for Mac. In general, I presume the Windows and Mac solutions could be quite different depending on the local minima the respective optimizers find, so it might be more useful to have tests with tight tolerances for each platform to ensure that each build continues behaving as expected. 🦓

[nickbianco]

I like the idea of having separate files for Mac and Windows (that would be helpful for some Moco tests too). The main issue I think is that the test seems to almost always fail on the first run and then almost always pass on the second run. Ideally we could identify what's different between those test runs.

But yeah, this isn't a big issue to begin with, mostly just annoying. I think raising the tolerance from 0.01 to 0.02 would let the CI pass; maybe that's reasonable enough.

[aymanhab]

My recollection is that there was some race condition where there's some dependency between this and other tests, so when run in parallel it fails but separately it passes. Disclaimer that was many many years ago when we looked into this so may not hold anymore.

[nickbianco]

Thanks @aymanhab. In that case, and for the sake of not spending too much time on this, I'm going to try updating the test to the catch framework and use the Simbody testing functions with some reasonable tolerances.

[nickbianco]

Update

Now that we've upgraded to Catch v3, I was able to implement support for platform-specific tests and a reasonable solution to prevent this test from always failing on CI. I discovered that running the test on Mac and Linux will somehow lead to CMC solutions with differing number of timesteps. The solutions are still reasonably close to the reference, so I interpolate the solution and use a looser test tolerance on these platforms. The Windows test is a stricter test that compares the solution to the reference element-by-element with a tighter tolerance.

If the cause for the spurious behavior on Mac/Linux is discovered, then these tests can be adjusted. But for now, this is my best attempt at reasonable test converage while not holding up CI everytime a new PR comes in. As a nice bonus, platform-specific testing is now available for all tests that use catch. I've updated CONTRIBUTING.md to summarize how to utilize platform-specific tests.

[aymanhab]

Was rather impossible to compare the std files on different platforms so I trust you assessed they are close enough. LGTM except for a minor comment, afterwards feel free to merge. Thank you

Reviewed 6 of 9 files at r3, all commit messages.
Reviewable status: 6 of 9 files reviewed, 2 unresolved discussions (waiting on @nickbianco)

---

CONTRIBUTING.md line 103 at r3 (raw file):

}

Thanks @nickbianco that's very helpful, I'd just add a note to discourage adding platform specific tests except for very specific cases as it would allow the platforms to diverge

---

OpenSim/Common/Test/testComponentInterface.cpp line 862 at r3 (raw file):

    REQUIRE_THROWS_AS(
        theWorld.finalizeFromProperties(),

Is this somehow related?

[nickbianco]

Thanks @aymanhab! Ready for review.

Yes, those files are very similar. Although the biggest improvement here is being able to interpolate the solution on Mac and Linux due to the behavior leading to different number of timesteps.

Reviewable status: 4 of 9 files reviewed, 2 unresolved discussions (waiting on @aymanhab)

---

CONTRIBUTING.md line 103 at r3 (raw file):

Previously, aymanhab (Ayman Habib) wrote…

Thanks @nickbianco that's very helpful, I'd just add a note to discourage adding platform specific tests except for very specific cases as it would allow the platforms to diverge

Done.

---

OpenSim/Common/Test/testComponentInterface.cpp line 862 at r3 (raw file):

Previously, aymanhab (Ayman Habib) wrote…

Is this somehow related?

Ah, debugging code I forgot to revert. Fixed.

[aymanhab]

Thanks @nickbianco

Reviewed 3 of 9 files at r3, 2 of 2 files at r4, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @nickbianco)