looser numeric tolerance for TestUtils.test_interface

[st--]

• fixes the docstring
• adds rtol specification where atol is also specified

[willtebbutt]

I'm happy for this to be merged, subject to resolving my comment, CI passing, and a patch bump being made.

[codecov]

Codecov Report

Merging #455 (d961d40) into master (033daf2) will not change coverage.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##           master     #455   +/-   ##
=======================================
  Coverage   93.18%   93.18%           
=======================================
  Files          52       52           
  Lines        1261     1261           
=======================================
  Hits         1175     1175           
  Misses         86       86           

Impacted Files	Coverage Δ
src/KernelFunctions.jl	100.00% <ø> (ø)
src/TestUtils.jl	94.11% <100.00%> (ø)

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 033daf2...d961d40. Read the comment docs.

[st--]

Looks like this hasn't resolved the test issue 😞 should we just bump the default rtol then ?

[devmotion]

Yeah that's what I expected unfortunately. According to the logs in the other PR for the failing tests rtol was already specified. I think the default values for the atol = 0 case would make sense, ie. rtol = sqrt(eps(T)). (Probably it would also make sense to use a T-dependent atol but could be done separately if needed at some point.)

[devmotion]

Is this change required? I would assume changing rtol to the default value for atol = 0 (i.e., sqrt(eps(T))) is sufficient. atol should only be needed when checking approximate equality involving an exact zero which I assume doesn't happen and I guess is fine with lower tolerances such as 1e-9.

[st--]

🤷 maybe not? I didn't think too much about it; we didn't carefully assess the tolerances to check whether atol=1e-9 is neeeded, or 1e-10 is enough... and sqrt(eps(Float64)) is ~2e-8 so not much of a change and it seemed nicely symmetric. Anyways, happy to change it back if (atol, rtol) = (1e-9, 2e-8) is better. What do you think @willtebbutt ?

[willtebbutt]

To my mind the point of the tests is to pick up on really substantial issues with a kernel implementation (i.e. substantial asymmetry in the kernel matrix, large negative eigenvalues), rather than to test that the numerics, as that presumably has to be a kernel-specific thing. Consequently, I'm happy with the looser atol that @st-- proposes.

@st-- is also correct that 1e-9 was chosen for no particular reason other than it seemed to satisfy the above in the cases we tested.

[st--]

ok, I'm going to add that as a comment though to make it easier to understand in the future