search

nlmixrdevelopment / nlmixr

nlmixr: an R package for population PKPD modeling
https://nlmixrdevelopment.github.io/nlmixr/
GNU General Public License v2.0
115 stars 45 forks source link

Make Tests Cross-Platform #99

Closed billdenney closed 5 years ago

billdenney commented 6 years ago

The tests currently don't all succeed on platforms other than Windows. It would be helpful if they ran successfully to very similar results (e.g. with a tolerance <5% and ideally <1%) across platforms. This discussion was started in RichardHooijmaijers/nlmixr.docker#2, but the cross-platform nature applies more to this repository (and, "Hi, @mattfidler" to make sure you see the connection here).

Initial testing succeeded with 196 tests and failed 11 (with an odd error that appears related to testthat and not nlmixr making me not certain that the numbers are everything). Your concerns. are correct; most values are <=3% off.

The only result that looked truly concerning that I saw off hand was from test-model68.R:

── 9. Failure: ODE (@test-model68.R#78)  ──────────────────────────────────────────
signif(as.numeric(z[4, "StdDev"]), 3) not equal to 0.000205.
1/1 mismatches
[1] 1.79e-07 - 0.000205 == -0.000205

(FYI, this was with my docker image not the stock image. Results could be slightly different in the stock docker image because of slightly different versions of the underlying libraries.)

My initial thought is that increasing the required precision for pnlsTol (e.g. to 0.01) may help.

mattfidler commented 5 years ago

Would you mind putting these results in separate files? (I can do it). That way if something changes I can use regular expression matching to remove the esitmation routines that may change.

What do you think?

billdenney commented 5 years ago

@mattfidler, that sounds like a good idea to me. I think that the files created would optimally be tagged with the test number and operating system.

(Sorry for my slow response.)

mattfidler commented 5 years ago

Great; Will proceed with this; Currently the other tests are tagged with the test number and the operating system.

mattfidler commented 5 years ago

I might break this out into a separate package and run just a very few models. These models with all the estimation routines make it too long to run on even a fast computer.

mattfidler commented 5 years ago

Hi @billdenney

Its been awhile. I will be breaking these tests into a vignette under nlmixr.examples

I found without this step, I really don't run the unit tests as often as I need to; Thank you for your contribution and understanding.

billdenney commented 5 years ago

Hey @mattfidler,

Sorry that I disappeared on this. Hopefully the provided code was helpful to taking this forward and improving the testing.

mattfidler commented 5 years ago

No Problem @billdenney

The code was useful, and was adapted for the vignette. I will run the vignette to see the performance before a CRAN release, but it will not be part of the required checks. It just takes too long.