test-workflow-bbi.R tweaks for upcoming Metworx

[kyleam]

• The first commit of this series reworks some assertions in test-workflow-bbi.R so that they pass on the upcoming Metworx (in addition to the current Metworx).

• The second commit was motivated by the upcoming Metworx but, due to changes on the Metworx side, is no longer strictly necessary. I think it's still a good change because it eliminates some cases where we'd need to make a bbr release to accommodate Metworx-side changes.

[kyleam]

@seth127 @barrettk These are the last pieces on my end that I'd like to get in before the release.

@seth127 I'm requesting your review since we've already discussed both of these, but please switch it over to @barrettk if that works better.

[barrettk]

@kyleam Just for additional background, is the tolerance now necessary on the upcoming Metworx BP because we added a new version of NONMEM? If not im curious why the results would differ more now

[kyleam]

is the tolerance now necessary on the upcoming Metworx BP because we added a new version of NONMEM?

That test is using nm74gf. As far as I know, there's no update to that version (e.g., a patch version).

If not im curious why the results would differ more now

When I say "[t]he expectation is too stringent because these values may change with different NONMEM versions or different systems" in the commit message, the "different systems" in this case is the OS update from Bionic to Focal (which includes, among many things, an update to gfortran). I think once you update the OS, all bets are off with respect to being able to produce the same values (see, for example, all the considerations for reproducing Stan results). If you or others have any NONMEM-specific expectations related to this, though, that's of course worth discussing so that we don't dismiss a discrepancy that we should look into more.

[barrettk]

the "different systems" in this case is the OS update from Bionic to Focal (which includes, among many things, an update to gfortran).

Had a feeling this was more of the concern, rather than an update to NONMEM. But awesome thanks for explaining.

I looked into the mpi_exec_path to get a better understanding of how/where it was being set. While I dont really understand what the last line is doing, I feel pretty good about the changes you introduced here in preparation for a potentially new mpiexec file location.

I'll approve from my end, but it sounds like Seth may still want to look at it given your comment about you two having discussed this previously.

[kyleam]

Looks good to me (left comments and questions in previous messages)

@barrettk Thanks for the discussion and review. I'll leave this open for a bit in case Seth wants to chime in, but from my view this is ready to go (and if you're at the point where you're ready to cut a release PR, please feel free to merge this).

[seth127]

This all looks good to me. Thanks both.

[kyleam]

Drone is currently not accepting any jobs. Holding off merge until it's resolved.