Unexpected inequality with large conversion factor

[inferiorhumanorgans]

Unexpected inequality with large conversion factor

[iliekturtles]

The slight difference in the values is a side effect of normalizing the internal value in meters and floating point imprecision. While PartialEq is implemented, I don't believe it will help in this case.

Floating point accuracy problems are always a hassle! You can see how uom's test code uses approx here:

uom/src/tests/mod.rs

Lines 110 to 140 in 629f385

	fn assert_eq(lhs: &Self, rhs: &Self) {
	match (lhs.is_nan(), rhs.is_nan()) {
	(true, true) => {}
	_ => { assert_eq!(lhs, rhs); }
	}
	}
	/// Assert that `lhs` and `rhs` are approximately equal for floating point types or
	/// exactly equal for other types.
	fn assert_approx_eq(lhs: &Self, rhs: &Self) {
	match (lhs.is_nan(), rhs.is_nan()) {
	(true, true) => {}
	_ => {
	assert_ulps_eq!(lhs, rhs, epsilon = EPS_FACTOR * V::epsilon(),
	max_ulps = ULPS);
	}
	}
	}
	/// Exactly compare `lhs` and `rhs` and return the result.
	fn eq(lhs: &Self, rhs: &Self) -> bool {
	(lhs.is_nan() && rhs.is_nan())
	|| lhs == rhs
	}
	/// Approximately compare `lhs` and `rhs` for floating point types or exactly compare
	/// for other types and return the result.
	fn approx_eq(lhs: &Self, rhs: &Self) -> bool {
	(lhs.is_nan() && rhs.is_nan())
	|| ulps_eq!(lhs, rhs, epsilon = EPS_FACTOR * V::epsilon(), max_ulps = ULPS)
	}

[inferiorhumanorgans]

Oh, whoops. I was the assert_relative_eq macro. The error was about not implementing the crate's RelativeEq trait.

[iliekturtles]

Are you proposing implementing approx::RelativeEq inside uom? If we're going to expose approx in uom's public interface I'd want to review other options. In a very brief scan I also found float-cmp and assert_approx_eq. All three seem pretty popular. I'm hesitant to pick a winner, but the orphan rules sort of require doing that.

[adamreichold]

I'm hesitant to pick a winner, but the orphan rules sort of require doing that.

Could you expand on why that is the case? Couldn't uom theoretically implement the traits from all three crates (and in different versions if required)? Also even if uom provides an implementation of only one trait, can't downstream crates still implement the other using newtype wrappers just like they have to do now for any of them? (Which would even allow overriding a provided implementation with the same effort as implementing it in the first place now.)

[iliekturtles]

I hadn't thought about the opposite and adding multiple implementations. Similar question though, when should the line be drawn on which external crates should be implemented?

[adamreichold]

Similar question though, when should the line be drawn on which external crates should be implemented?

Where you as the maintainer want it to be drawn from an effort versus convenience perspective. Since the basic set of implementable traits is unaffected, the stakes for answering this are not that high, i.e. you can choose one that you like to provide a convenient path for some users while not blocking anything for others.

Of course, just saying no is also a maintainer's prerogative and providing none also does not block anyone from using newtype wrappers. My point basically is that the orphan rules only enforce one thing here: This crate is the only place where we can provide these trait implementations without the additional effort of newtype wrappers.