jtempest / float_eq-rs

Compare IEEE floating point values for equality.
Other
39 stars 6 forks source link

Use with Complex<T> should be clarified #23

Closed blueglyph closed 2 years ago

blueglyph commented 2 years ago

Problem

I need to compare Complex64 values and to assert their equality in unit tests. I'm at a loss when I'm trying to use the macros with complex values. It looks like the documentation is incorrect, and that some items that should be public are actually not public.

Step

On https://crates.io/crates/float_eq, this example shows how to do the comparison by using the :

let a = Complex32 { re: 2.0, im: 4.000_002 };
let b = Complex32 { re: 2.000_000_5, im: 4.0 };

assert_float_eq!(a, b, ulps <= Complex32Ulps { re: 2, im: 4 });

(1) I couldn't find Complex32Ulps. However, there is a ComplexUlps32 so I assume it is a typo. This page suggests it is public as float_eq::ComplexUlps32. But when I try, it fails to compile because of an unresolved import.

(2) By looking at the source code in src/trait_impls/num_complex.rs, I see ComplexUlps32 is defined as public, but it doesn't look like it can be used outside of the crate (and I haven't seen any public import).

(3) I resorted to re-defining it myself. Also, it cannot be instantiated as a structure, so I used ComplexUlps32::new instead.

        use float_eq::{float_eq, assert_float_eq};
        type ComplexUlps32 = UlpsTol<Complex<f32>>;
        let a = Complex32 { re: 2.0, im: 4.000_002 };
        let b = Complex32 { re: 2.000_000_5, im: 4.0 };
        assert_float_eq!(a, b, ulps <= ComplexUlps32::new(2.0, 4.0));

but then I get this error:

error[E0277]: the trait bound `num::Complex<f32>: FloatEqUlpsTol` is not satisfied
   --> rpnc-lib\src\value.rs:398:40
    |
398 |         assert_float_eq!(a, b, ulps <= ComplexUlps32::new(2.0, 4.0));
    |                                        ^^^^^^^^^^^^^ the trait `FloatEqUlpsTol` is not implemented for `num::Complex<f32>`

I'll continue searching a little bit, but either I'm misusing it, or something is wrong in the crate. How can one use assert_float_eq and float_eq with Complex<T>?

Version

float_eq = "1.0.0" rust toolchain 1.62.0 used with Rust edition 2021

blueglyph commented 2 years ago

It's really strange. If I try to use float_eq or assert_float_eq on Complex64 directly:

        let a = Complex64 { re: -12.0, im: 16.0 }.sqrt();
        let b = Complex64 { re: 2.0, im: 4.0 };
        assert_float_eq!(a, b, r2nd <= Complex64::new(1.0 * f64::EPSILON, 2.0 * f64::EPSILON));
or
        let a = Complex64 { re: -12.0, im: 16.0 }.sqrt();
        let b = Complex64 { re: 2.0, im: 4.0 };
        let comp = float_eq!(a, b, r2nd <= f64::EPSILON);
        println!("a={}, b={}, cmp={:?}", a, b, comp);

I get a similar error:

the trait FloatEq<_> is not implemented for Complex<f64>

Yet FloatEq is implemented for Complex<T> in src/trait_impls/num_complex.rs. And I think f64 qualifies as T since it's Sized and I saw an implementation of FloatEqUlpsTol (requirement for FloatEq::Tol).

Of course I cannot implement it myself since Complex<T> is not annotated as fundamental.

blueglyph commented 2 years ago

Finally, if I implement FloatEq manually for a custom struct or enum, is it necessary to implement it manually for Option and Result of that struct / enum? It looks like it should be done automatically, but it doesn't (impl<T: FloatEq> FloatEq for Option<T> in core_types.rs.

jtempest commented 2 years ago

Hello! The num::Complex support is an optional feature, although that isn't communicated very well so I'll think about changing the docs. You can enable it like so:

[dependencies]
float_eq = { version = "1", features = ["num"] }

As for Option<T>, If you've implemented the relevant float_eq traits on T, it should have that support for free. Result<T> is not supported by float_eq's traits and likely won't be - heterogeneous types don't really work with the model I built the macros on, unfortunately.

jtempest commented 2 years ago

Oh, and I think you're right about Complex32Ulps being a typo and the library refering to it as ComplexUlps32 too :)

blueglyph commented 2 years ago

I did try with ComplexUlps32 (or 64), but it gave the error above, I couldn't import it with a use. So I implemented FloatEq by comparing both real and imaginary parts.

It didn't work with Option, I got the error above, but I only implemented FloatEq along with a lot of other traits & structures (ValueDebugUlpsDiff, FloatEqDebugUlpsDiff, ValueUlps, FloatEqUlpsTol, AssertFloatEq ...) which are required to avoid panics with the assert macro in some circumstances, even though I'm only using assert_float_eq! with eq_r2nd.

Besides, I have no idea what to do with functions like debug_r2nd_tol to allow for errors which lead to non-sensical comparisons like a string with an f64 cases of the enum, for example. I assume this particular use-case is not supported (since it's meant for struct, not enum). Even though, it's a lot of code for just an assert so I find it strange, I must have missed something.

I'll probably have to restrict the basic use to float_eq! for identical enum cases carrying f64/Complex, manage Option / Result manually, and comparing different enum cases manually too.

jtempest commented 2 years ago

Did you enable the "num" feature? I've just tried this program with float_eq 1.0.0 and it compiles and runs:

use float_eq::{assert_float_eq, ComplexUlps64};
use num::Complex;

fn main() {
    let a = Complex::<f64> { re: 1.0, im: 1.0 };
    let b = Complex::<f64> { re: 1.0, im: 1.0 };
    assert_float_eq!(a, b, ulps <= ComplexUlps64 { re: 1, im: 2 });
}

With this Cargo.toml:

[package]
name = "float_eq_test"
version = "0.1.0"
edition = "2021"

[dependencies]
float_eq = { version = "1", features = ["num"] }
num = "0.4"
blueglyph commented 2 years ago

Did you enable the "num" feature?

No, you're right. I did try something and it didn't work, and I focused on the guide and API, where I may have missed it. It's clear enough in the README though, so it's on me. :)

If I enable it, this error disappears. Thanks!

blueglyph commented 2 years ago

And I found out about the Option issue:

For actual: Option<T> and expected: T, this doesn't work:

assert_float_eq!(Some(expected), actual, r1st <= f64::EPSILON, "test #{} failed", idx);

This does:

assert_float_eq!(Some(expected), actual, r1st <= Some(f64::EPSILON), "test #{} failed", idx);

Now that I've tried it's obvious, but it wasn't very intuitive because of the r1st <=. I first thought the Option was unwrapped on the first two data, and there were other unknowns when I tested it so I stopped there.

jtempest commented 2 years ago

I've fixed up the documentation so the typenames are correct now for 1.0.1, please let me know if you have any further trouble :)

blueglyph commented 2 years ago

It's been a while; I suppose it's fine though one must make sure not to miss the "other optional features" bit for composite types like Complex. I'd definitely mention it where appropriate to avoid any confusion.

Thanks!