phenofit icon indicating copy to clipboard operation
phenofit copied to clipboard

Numerical tolerance of test doesn't allow for reasonable exp inaccuracy.

Open kalibera opened this issue 8 months ago • 0 comments

The CRAN version of this package (0.3.9) unfortunately fails its tests on Windows with mingw-w64 v12. Rtools45 still uses mingw-w64 v11 to allow time for fixing packages to work also with v12.

The problem in phenofit is that reduced accuracy of function exp() in mingw-w64 v12 (v12 uses a number of Math functions from UCRT instead of a custom replacement) leads to a result no longer within the tolerance limit. However, the reduced accuracy is still within 1 ULP, which is something applications need to be able to accept.

The CRAN version of the package can be updated as follows to pass the tests:

diff -Nru orig/phenofit/tests/testthat/test-curvefit.R patched/phenofit/tests/testthat/test-curvefit.R
--- orig/phenofit/tests/testthat/test-curvefit.R        2022-10-28 13:00:08.000000000 +0200
+++ patched/phenofit/tests/testthat/test-curvefit.R     2025-04-23 15:47:55.844132300 +0200
@@ -15,7 +15,7 @@
 test_that("curvefit works", {
     p1 = get_param(fit_cpp)[1:5]
     p2 = get_param(fit)[methods][1:5]
-    expect_true(all.equal(p1, p2, tolerance = 1e-6))
+    expect_true(all.equal(p1, p2, tolerance = 1e-5))
     expect_silent(r <- get_param(list(fit)))
 
     # For Klos, the result of C++ is slightly different from that of R version.

This is a subset of a change that has been already made in phenofit, https://github.com/eco-hydro/phenofit/commit/f109af1345995546c79567fd8e323e901a1309b1, so no further code changes seem to be needed, but it would be great if a new release to CRAN could be made with this fix.

kalibera avatar Apr 23 '25 16:04 kalibera