Question: Default equality precision

[dschuermans]

Hey,

I'm wondering if there's anything done concerning default equality functionality...
From the docs i found that precision is being tested up to 1E-5.

However, when doing the following:

Length unit2 = Length.FromFeet(10D * 3.28084D)

Assert.AreEqual(unit1, unit2)
Assert.IsTrue(unit1.Equals(unit2)

the test fails.

However:
unit1.Equals(unit2, 1E-5, ComparisonType.Absolute)
will return true.

The value in meters for unit2 is 10.00000032

Should we always use the overloaded Equals methods or is there a way we can configure the default Equals?

[angularsen]

I believe we discussed using ComparisonType.Relative as the default one with a sane value, but I don't recall if we found some use cases where that would not work as expected or not. If you want to look into overriding the default Equals(), start with searching through old issues to see if it's been discussed before. If we can find a behavior that works for a range of small and large quantities and some unit tests on it, then I'm all for changing it. Best, Andreas

[tmilnthorp]

I would argue that as a value type, Equals should work similar to how you would expect with Double.Equals. It does not return true for anything other than absolute equality.

Looking at our Equals(object) implementation, it checks of the object is the same quantity, converts it to the same unit, and checks for absolute equality. As a value type, we probably shouldn't even do the conversion. It should check that the value and the unit are exactly the same.

[lipchev]

I agree with @tmilnthorp - this is also how the GetHashCode function is currently implemented (I meant to open issue on the subject)- we could alternatively devise an implementation of GetHashCode that is consistent- by converting (and possibly rounding to tolerance- should this become the default equality contract) - but I think it should go into the UnitsNet.Comparison class rather than as the default implementation.
It is then up to the domain class that holds the measure units to re-define Equals/HashCode, should this be deemed appropriate: one example (from my experience) is the chemical solution- still a value object(no identity) but considered equal to another solution with the same component(s) and concentration(s) within a margin of error- specified by the domain experts.

[angularsen]

My first reaction also is that it should use double.Equals ("double-equals" from now on) and perhaps even compare both Value and Unit with no conversions, similar to Tuple<double, Enum> and anonymous type equality new { Value, Unit }.Equals() ("tuple-equals" from now on).

It feels more correct.
However, I don't think this design is terribly useful. At least the way I am using this library.

I don't really have a strong opinion either way, but here is my point of view as a consumer of the library.

Comparing unit + value

I don't immediately see a scenario where tuple-equals is something I want when comparing quantities. Typically I want to compare quantities for their amount of something - regardless of the unit it happened to be constructed with.

Do you have any examples where you think tuple equality would be more intuitive?

Comparing floating precision values

If we had used int or long as backing value then fine, compare it exactly like TimeSpan and DateTime does with their long Ticks value.

Since we are using double, then I don't find floating point equality checks very useful - because they have unpredictable rounding errors. The ReSharper plugin will even warn you if you try to compare a float or double with some numeric value.

Also, from the consumer perspective; the fact that we are using double internally is really an implementation detail. We could have used decimal if we wanted, or fixed precision with long or similar. I don't think it is fair to expect the consumer to know and understand that our equality methods are prone to subtle internal rounding errors.

For example:

Length l1 = Length.FromMeters(1.123);
Length l2 = Length.FromMeters(2.456);
Length l3 = Length.FromMeters(3.579);

// False
(l1 + l2 == l3).Dump("l1 + l2 == l3");

Here the equality is false, but I know this can trip people up due to the miniscule rounding errors that would occur for these particular choice of values. Other values may work just fine. It is unpredictable.

Proposal: Relative comparison with 1e-5 max error

My argument is that it might be an improvement for the default equality to allow for some rounding errors. The Equals and == operator would then actually be useful for something.

I have no idea what comparison parameters would be a good default choice, or if this even works, but I thought maybe 1e-5 max relative error would be a good starting point?
In theory it should work for both small and large quantities, but we'd need to run some tests to conclude more.

(l1 + l2).Equals(l3, 1e-5, ComparisonType.Relative).Dump(); // True

What do you think?
Can it be a way to go or are there maybe advantages with double-equals or tuple-equals I am missing?
Will it not work for certain values or units?

[tmilnthorp]

Remember that if Equals returns true, GetHashCode must return the same value for the two equal objects. Otherwise any hash based structure such as Dictionary would fall apart. Absolute equality is essential.

We could create a custom IEqualityComparer though, which would make the fact you're using relative equality explicit.

[angularsen]

That is a very good point. One that is enough to put all my above arguments to rest.

I will say, however, due to the floating point issues I would never use an IQuantity as the key of a dictionary or in a Hashset or similar. It just seems like a bad idea, but at least if we implement the tuple equality (or keep the current double equality) then at least it would be consistent in such hash structures. I can go with that.

Good point about the IEqualityComparer, that is something we could provide paired up with our Equals(other, error, comparisonType) overload methods.

[lipchev]

I don't think your arguments are invalidated by the hash-equals contract requirement. It's just a matter of rounding off the value used for the hash calculation with the same tolerance (or possibly using an even larger tolerance - as hash equality can be weaker) as the one used for the equals method. The question is "what should be the default contract?". PS It does seem like a bad idea to use an IQuantity as the key by itself, but it could make sense to use it as part of a larger (value) object- like in my example above.

…

On Thu, 31 Oct 2019 09:26 Andreas Gullberg Larsen, ***@***.***> wrote: That is a very good point. One that is enough to put all my above arguments to rest. I will say, however, due to the floating point issues I would never use an IQuantity as the key of a dictionary or in a Hashset or similar. It just seems like a bad idea, but at least if we implement the tuple equality (or keep the current double equality) then at least it would be consistent in such hash structures. I can go with that. Good point about the IEqualityComparer, that is something we could provide paired up with our Equals(other, error, comparisonType) overload methods. — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#717?email_source=notifications&email_token=ABICQQEJBZFGQ7FD6M7JN7DQRKCCZA5CNFSM4JGXOEKKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOECWZJSA#issuecomment-548246728>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ABICQQEXFVHJGE2LXO27CDTQRKCCZANCNFSM4JGXOEKA> .

[tmilnthorp]

HashSet may have been a better example - for set operations. It too supports an IEqualityComparer to be passed in.

[angularsen]

@lipchev

It's just a matter of rounding off the value used for the hash
calculation with the same tolerance

If I understand correctly, you propose dividing the values into ranges/buckets that produce the same hashcode?

Consider this example:

var l1 = Length.FromMeters(1);
var l2 = Length.FromMeters(1 + 0.1);
var l3 = Length.FromMeters(1 - 0.1);

// Let's say allowed relative error is 0.1 and these are true
l1.Equals(l2); 
l1.Equals(l3);

// But this is false (relative error is ~0.2)
l2.Equals(l3);

// Let's say we compute hashcode by rounding value to nearest 0.1
l1.GetHashCode().Equals(l2.GetHashCode());

// Becomes
(1.0 hashcode).Equals(1.1 hashcode); // False

// If we choose a wider rounding value 0.2, it still fails
(1.0 hashcode).Equals(1.2 hashcode); // False

// If we choose even wider rounding value 0.5 then finally the
// hashcodes are equal and matches the Equals() behavior that allowed 0.1 error
(1.0 hashcode).Equals(1.0 hashcode); // True

Maybe I am misunderstanding, but it seems tricky to get the choice of rounding value right.

At any rate, I am more inclined to provide good IEqualityComparer implementations instead. It is more explicit and I think we will avoid a lot of weird edge-cases trying to implement a default Equals() method. We also already have Equals() overloads that take parameters to control the equality behavior, so that aspect is somewhat discoverable except for maybe when writing unit tests and using Assert.AreEquals(). I think I'm leaning towrds keeping it as-is, but let me know if you see it differently! Maybe there is a good compromise lurking here.

[lipchev]

I tried out a few functions in excel- and managed to get some segments(centered @ zero) with ranges of 2, 1 and 0.5 respectively- but couldn't quite figure out a general formula. Note however that for the last one it is true that for any number input around the integer numbers (say a nano-meter) the hash generated for every length in the range of +/- 0.25 of a nano-meter. So my intuition tells me that there is a function that should work for the absolute difference. And likely even one that works with integer arithmetic. Any ideas? Also cool would be to have a percent rounding equivalent. Finally a little clarification about the equals/hash contract:

Two objects that are equal return hash codes that are equal. However, the reverse is not true: equal hash codes do not imply object equality, because different (unequal) objects can have identical hash codes.

[lipchev]

[angularsen]

Cool figures, but I am a bit on deep waters here and don't fully understand this enough to provide any ideas to help you move this forward 🤷‍♂ 😄

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.