-
Notifications
You must be signed in to change notification settings - Fork 20
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
Tolerance applies in a surprising way to large integers #138
Comments
I'm not sure how to resolve this; the |
IMO the
Time-since-UTC-epoch is an implementation detail to which we shouldn't over-anchor ourselves. Any timestamps that are 5 seconds apart should have the same output of |
I agree that any of timestamps that are (say) >0.01s different should compare as equal. But I'd think that same principle should also apply to integers? (regardless of whether they're integers or doubles under the hood). Why should the tolerance for only POSIXct always be absolute? |
Because time is "interval data" which for which 0 is arbitrary; temperature should similarly get its own (struggling to find something authoritative-looking about this, so not sure the term is universal, but here's one source) For unclassed numerics, it's a significant figures issue, right? |
I don't think it's about significant digits as much as floating point differences like My intuition (which might be wrong) is that the tolerance should really only affect very small numbers and very very large numbers (i.e. where the exponent is very large and the gap between representable floating point numbers is larger), but numbers in the middle shouldn't be affected. Maybe I'm expecting the tolerance to be applied to the mantissa? (assuming the exponents must be equal) |
I think I'm maybe thinking of ULP based comparison? https://bitbashing.io/comparing-floats.html has a good discussion. https://codingnest.com/the-little-things-comparing-floating-point-numbers/ is also useful because it describes what Catch2 (a C++ testing framework does) |
I think overall this implies that waldo's current approach to tolerance is overly simplistic and will need to be redesigned (in conjunction with testthat) to allow difference classes to use different values. That will require some thought so I think I'll at least get the current release out before coming back to think about this. |
That makes more sense to me, thanks for elaborating & thanks for the links. One lingering thing that gives me pause is a potential "idempotency" issue? I'm thinking of large (I'm not 100% sure it's a relevant concern -- it could be that the imprecision happens only at the sub-integer level for numbers in integer range) |
Created on 2022-03-12 by the reprex package (v2.0.1)
Compared with
all.equal()
which has different behaviour for POSIXct:Created on 2022-03-12 by the reprex package (v2.0.1)
I don't think the behaviour is correct in either case: the purpose of tolerance is to ignore minor differences that arise due to floating point error, not to ignore differences in integers that can be exactly represented in floating point.
cc @MichaelChirico
The text was updated successfully, but these errors were encountered: