approx icon indicating copy to clipboard operation
approx copied to clipboard

relative_eq() has incorrect behavior for small values

Open dsroche opened this issue 4 years ago • 3 comments

Consider this small example:

let diff: f32 = 1.0e-7;
let x: f32 = 1.0e-5;
let y = x - diff;
assert_ne!(x, y);
relative_eq!(x, y) // returns true, but should be false

Note that x and y are normal, full-precision floating-point values (not "subnormals"), above the epsilon threshold, and their relative error is (x - y)/x, which equals 0.01, far above the default threshold. However, they are marked as "relatively equal" in the current implementation.

The problem is in the logic of the provided implementation of RelativeEq::relative_eq() for f32 and f64, namely these lines:

https://github.com/brendanzab/approx/blob/308176d3d882c75dd9b57d9a7711e879a76119a8/src/relative_eq.rs#L65-L70

While the precise definition of relative equality and the parameters epsilon and max_relative are never clearly specified in the documentation, based on the referenced blog posts, it seems that the intention is to use epsilon as a zero-closeness threshold, and max_relative as the relative error threshold.

The issue stems from not checking the two input numbers (self and other in this case) individually against epsilon for a zero-threshold, but instead testing their absolute difference against epsilon. As a result, relative equality works as expected for large numbers, but in practice defaults to absolute-difference equality for values less than 1.

dsroche avatar Feb 01 '21 00:02 dsroche

Rust follow IEEE 754 all is specified here. (but I have no idea if this will answer you)

Stargateur avatar Mar 11 '21 00:03 Stargateur

@dsroche does the code below illustrate your point? I'm not that familiar with relative floating point comparisons:

fn relative_eq(lhs: f32, rhs: f32, epsilon: f32, max_relative: f32) -> bool {
    // Handle same infinities
    if lhs == rhs {
        return true;
    }

    // Handle remaining infinities
    if f32::is_infinite(lhs) || f32::is_infinite(rhs) {
        return false;
    }

    let abs_diff = f32::abs(lhs - rhs);

    // For when the numbers are really close together
    if abs_diff <= epsilon {
        return true;
    }

    let abs_lhs = f32::abs(lhs);
    let abs_rhs = f32::abs(rhs);

    let largest = if abs_rhs > abs_lhs { abs_rhs } else { abs_lhs };

    println!("This part is never reached");
    // Use a relative difference comparison
    abs_diff <= largest * max_relative
}

fn main() {
    let diff = 1.0e-7;
    let x: f32 = 1.0e-5;
    let y: f32 = x - diff;

    println!(
        "relative_eq!(x, y) = {}",
        relative_eq(x, y, f32::EPSILON, f32::EPSILON) // true
    );

    println!(
        "abs_diff <= largest * max_relative = {}",
        (x - y).abs() <= 1.0e-5 * f32::EPSILON, // false
    );
}

So one has to pass their own epsilon in order for it to work for small values? E.g:

use approx::relative_eq;

relative_eq!(x, y, epsilon = 1.0e-8f32)

imjasonmiller avatar Mar 11 '21 14:03 imjasonmiller

Oooh thanks for the report! I may have got something wrong in the implementation - it's been a long time since I looked inside. And to be honest floating point numerics are not something I'm an expert on. I'd be interested if we can find a fix for this though!

brendanzab avatar Mar 23 '21 08:03 brendanzab