librascal
librascal copied to clipboard
constexpr static TOLERANCE in files to be used from rascal::math for unification
In the half and full neighbourlist adaptor this constexpr variable pops up. I'm gonna redirect that to the math definitions to be consistent.
I am not sure we want to unify math::DBL_FTOL
and the tests TOLERANCE
.
One is for internal use inside librascal implementation, the others are test-specific tolerances. Different tests already use different tolerances, from 1e-10
to 1e-14
.
math::DBL_TOLERANCE
is defined as 100.0 * std::numeric_limits<double>::epsilon()
, which is 100 * 2.22045e-16
, so roughly 2e-14
.
I am also not convinced by the need for a single tolerance value for all of rascal functions, and why it is set to this specific value. std::numeric_limits<double>::epsilon()
is defined as the distance between 1.0 and the next representable double
, and thus refer to the rough precision of double around 1.0. But this precision is not the same across the whole range of double values.
I think we should in general use relative tolerances (abs((value - reference) / reference) < TOL
) instead of absolute tolerances (abs(value - reference) < TOL
), since they make more sense and are uniform around the range of double values. We might also want to look into ULPs (units of least precision), since they are designed for floating point comparison.
See these very nice articles for more information on floating point values comparison: https://bitbashing.io/comparing-floats.html, https://randomascii.wordpress.com/2012/02/25/comparing-floating-point-numbers-2012-edition/
Well, you make a good case, can't argue with that.
Then I'll reformulate that issue to change double comparisons to relative double comparisons and keep the local tolerance?
On Nov 10, 2019 20:37, Guillaume Fraux [email protected] wrote:
I am not sure we want to unify math::DBL_FTOL and the tests TOLERANCE.
One is for internal use inside librascal implementation, the others are test-specific tolerances. Different tests already use different tolerances, from 1e-10 to 1e-14.
math::DBL_TOLERANCE is defined as 100.0 * std::numeric_limits
I am also not convinced by the need for a single tolerance value for all of rascal functions, and why it is set to this specific value. std::numeric_limits
I think we should in general use relative tolerances (abs((value - reference) / reference) < TOL) instead of absolute tolerances (abs(value - reference) < TOL), since they make more sense and are uniform around the range of double values. We might also want to look into ULPs (units of least precision), since they are designed for floating point comparison.
See these very nice articles for more information on floating point values comparison: https://bitbashing.io/comparing-floats.html, https://randomascii.wordpress.com/2012/02/25/comparing-floating-point-numbers-2012-edition/
You are receiving this because you were assigned. Reply to this email directly, view it on GitHubhttps://github.com/cosmo-epfl/librascal/issues/210?email_source=notifications&email_token=AA44BXXRQIDK5U7UMQWIVM3QTBPC5A5CNFSM4JLNRZC2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEDVE24A#issuecomment-552226160, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AA44BXW44HZ4PSSDSASLGO3QTBPC5ANCNFSM4JLNRZCQ.
Was this addressed in a recent PR (#196 perhaps) and, if so, can we close this?
No, this one is referring to the tests as far as I remember, Guillaume is right with his comment.