-
Notifications
You must be signed in to change notification settings - Fork 10
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
Add float matchers #101
Comments
Perhaps something like: template<typename T>
struct within {
T expected = {};
struct tolerance_thresholds {
T abs = {};
T rel = {};
} tolerance;
explicit within(T e, T abs_tol) : expected(e), tolerance{.abs=abs_tol} {}
explicit within(T e, const tolerance_thresholds& tol) : expected{e}, tolerance{tol} {}
constexpr bool match(T value) const noexcept {
const T tol = std::max(tolerance.abs, tolerance.rel * std::max(std::abs(value), std::abs(expected)));
return std::abs(value - expected) <= tol;
}
}; REQUIRE_THAT(value, WithinAbs(expected, 1e-6)); // Catch2
REQUIRE_NEAR(value, expected, 1e-6); // GoogleTest
REQUIRE_THAT(value, within{expected, 1e-6}); // snitch
REQUIRE_THAT(value, within{expected, {.abs=1e-6}}); // snitch
REQUIRE_THAT(value, WithinRel(expected, 1e-3)); // Catch2
REQUIRE_THAT(value, within{expected, {.rel=1e-3}}); // snitch
REQUIRE_THAT(value, WithinRel(expected, 1e-3) || WithinAbs(expected, 1e-6)); // Catch2
REQUIRE_THAT(value, within{expected, {.abs=1e-6, .rel=1e-3}}); // snitch
REQUIRE_THAT(value, WithinRel(expected, 1e-3) && WithinAbs(expected, 1e-6)); // Catch2
// Until we implement combining matchers:
REQUIRE_THAT(value, within{expected, {.rel=1e-3}}); // snitch
REQUIRE_THAT(value, within{expected, {.abs=1e-6}}); // snitch
// When we implement combining matchers:
REQUIRE_THAT(value, within{expected, {.rel=1e-3}} && within{expected, {.abs=1e-6}}); // snitch |
Here are two concerns about the proposed design:
|
Thanks for the feedback!
My limited study so far seems to indicate that the most common need is to just have an absolute tolerance. And at work, we either use absolute tolerances, or both absolute and relative (with an OR combination). A relative tolerance on its own is usually impractical, because it can reduce to a ridiculously small tolerance for small numbers, and it usually ends up far lower than numerical precision. In this case, we can be just a few character longer than Catch2 REQUIRE_THAT(value, WithinAbs(expected, 1e-6)); // Catch2
REQUIRE_THAT(value, within{expected, {.abs=1e-6}}); // snitch or a few character shorter if we add a special case for this: REQUIRE_THAT(value, WithinAbs(expected, 1e-6)); // Catch2
REQUIRE_THAT(value, within{expected, 1e-6}); // snitch I think this is the approach I am aiming for:
That's true, but snitch being a C++20 library, I don't think it's a stretch to rely on this. Designated initializers are an awesome readability feature, we should use it and encourage it when it helps. Although a user could choose not to use them and end up with
Yes I agree. I wanted to rename it to Ideas that came to mind while writing this; will have to get back to them later when I have time: Generic
|
Hmm indeed, REQUIRE_THAT(value, WithinAbs(expected, 1e-6)); // Catch2
REQUIRE_THAT(value, within{expected, absolute{1e-6}}); // snitch
REQUIRE_THAT(value, within{expected, 1e-6}); // snitch (shortcut, for most commonly used form)
REQUIRE_THAT(value, WithinRel(expected, 1e-3)); // Catch2
REQUIRE_THAT(value, within{expected, relative{1e-3}}); // snitch
REQUIRE_THAT(value, WithinRel(expected, 1e-3) || WithinAbs(expected, 1e-6)); // Catch2
REQUIRE_THAT(value, within{expected, relative{1e-3} || absolute{1e-6}}); // snitch
REQUIRE_THAT(value, WithinRel(expected, 1e-3) && WithinAbs(expected, 1e-6)); // Catch2
REQUIRE_THAT(value, within{expected, relative{1e-3} && absolute{1e-6}}); // snitch In terms of verbosity, we've lost most of the advantages... I really like the concept of the generic Here's another example of what we could do with this, for the physicists among us. The following cannot be done with Catch2 builtin matchers, you would have to write your own: // Adding overload for generic tolerance functions (this would be added to snitch)
template<typename T>
concept tolerance_function = requires(const T& func, double v) {
{ func(v) } -> std::convertible_to<double>;
};
template<floating_point T, tolerance_function F>
T get_abs_tolerance(T value, const F& func) {
return func(value);
}
// Usage:
CHECK(1.001f == within{1.0f, [](auto v) { return std::sqrt(1e-12 + v * v * 1e-6); }});
// Or in a more real-world setting:
const auto computed_mass = ...;
const auto expected_mass = ...;
const auto mass_error = [](auto v) { return std::sqrt(1e-12 + v * v * 1e-6); };
CHECK(computed_mass == within{expected_mass, mass_error}); The one thing I'm not sure of is the computation of the "magnitude" as REQUIRE_THAT(1.0f, within(1.1f, 0.1f));
REQUIRE_THAT(1.1f, within(1.0f, 0.1f)); // identical as line above I'm not sure this is actually a desirable property. The expected value is the one that matters for setting the magnitude of the tolerance, regardless of what value was actually measured. I might change this to |
A brief review of floating point testing practices in popular open source libraries:
|
Parking this for a bit; the latest prototype can be found in https://github.com/cschreib/snitch/tree/float_matcher. |
It is a desirable property because it is an expected property of the comparison when dealing with numerical comparisons in literature. This is not the same as being intuitive and I agree that
It is a copy of old version of Approx, which had some weird ideas (no margin, non-zero scale default, etc) |
So I think it all boils down to terminology; perhaps Edit: As an example of this argument: "x must be within +/- 5% of y" --> "+/- 5% of y" should unambiguously evaluate to an acceptable error range of |
Catch2 offers some builtin float matchers to compare two floating point numbers for approximated equality. This seems to be a popular feature:
It could be easy to include without bloating snitch.
As stated in the discussions linked above, we first need to understand what people are generally trying to achieve. There are a lot of misconceptions around floating point numbers, and some traps that are easy to fall into.. Something too simple might not fit anybody's needs. We could also quickly over-engineer this in order to make it generally useful to everyone, and we should try to avoid that.
Catch2 used to offer the
Approx
matcher, but this was eventually deprecated in favor of theWithinAbs
/WithinRel
/WithinULP
matchers. If we are not going to just port over the new Catch2 matchers, we should learn from this. The weaknesses ofApprox
were:double
internally no matter the type of the input.a == Approx(b)
isn't the same asb == Approx(a)
).The new matchers presumably fix all this, however they rely on combining matchers to work nicely, so we would have to implement this as well:
I think this is a nice idea, it allows specifying pretty much all combinations of tolerances one may need. However it is a bit verbose, we can probably improve on that.
doctest uses
Approx
, I believe it is the same as the one in Catch2.GoogleTest uses
EXPECT_NEAR
, which is just an absolute tolerance check. They have various matchers although they appear to only support absolute tolerances.The text was updated successfully, but these errors were encountered: