ChainRulesTestUtils.jl icon indicating copy to clipboard operation
ChainRulesTestUtils.jl copied to clipboard

default test tolerances assume Float64 precision and numbers ~ 1

Open stevengj opened this issue 3 years ago • 1 comments

The default test tolerances here seem wrong: https://github.com/JuliaDiff/ChainRulesTestUtils.jl/blob/6925da14c12e3d743c8d3620db8a8bee1433d5c3/src/testers.jl#L17

  1. The default rtol = 1e-9 assumes double precision. Better to follow isapprox here and use sqrt(eps(float(T)))?
  2. By setting a nonzero default atol you are implicitly assuming quantities of order unity. This corresponds to an absurdly large tolerance if the functions happen to be scaled to have very small magnitude. Better to follow isapprox here and default to atol = 0.

stevengj avatar Jan 31 '23 22:01 stevengj

This makes sense. It should be a negligible change, and we can use the type of the input to determine the type we should use for rtol, at least for the test_scalar case. Might be a little more fiddly for the test_rule and test_frule.

We should check if this breaks anything downstream, but assuming it doesn't happy enough to see this in a bug fix release. Otherwise we might have to do more thinking

oxinabox avatar Mar 08 '23 14:03 oxinabox