Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Increase tolerance for compare() when long doubles aren't available #940

Closed
richierocks opened this issue Sep 27, 2019 · 3 comments
Closed

Comments

@richierocks
Copy link

CRAN recently started testing packages on machines where long double aren't available. This can cause broken tests that require many package developers to spend time scouring the internet for a solution.

There's a nice overview of the issues and solution here: https://blog.r-hub.io/2019/05/21/nold

It seems that many problems would disappear with a small change to testthat::compare().

Currently in the signature to testthat:::compare.numeric, tolerance defaults to .Machine$double.eps^0.5.

I think a better default is .Machine$double.eps ^ if(capabilities("long.double")) 0.5 else 0.25.

If silently increasing the tolerance worries you, it could be done with a message.

get_default_tolerance <-  function() {
  pwr <- if(capabilities("long.double")) {
    0.5
  } else {
    message("Long doubles are not available, so tolerance is increased.")
    0.25
  }
  .Machine$double.eps ^ pwr
}

Then the argument default would be tolerance = get_default_tolerance().

@hadley
Copy link
Member

hadley commented Sep 27, 2019

I think noLD is sufficiently unusual in practice that I don't really care. I think it would also be ok to do something like this:

default_tolerance <-  function() {
  if (!capabilities("long.double")) {
    skip("Long doubles not available and no tolerance supplied")
  }

  sqrt(.Machine$double.eps)
}

@richierocks
Copy link
Author

The problem I was trying to solve is that CRAN now enforces all tests passing in an environment without long doubles, but many packages developers don't know what changes they need to make if their tests fail.

If I've understood your default_tolerance() solution correctly, it means that package developers will still have to write custom logic for the noLD case to explicitly pass a tolerance argument.

My solution was based around "set a tolerance that should allow tests to pass when they are run in environments with or with LDs". I think it should help avoid having to write boilerplate logic.

@hadley
Copy link
Member

hadley commented Sep 27, 2019

If they don’t supply an explicit argument, the test will be skipped which seems fine to me (I don’t particularly like changing constants without some convincing analysis)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants