Change the underlying Value type from Double to Fraction (WIP)

[lipchev]

As discussed in #1367 (and many times before) - this is an attempt to solve the rounding issues with the Equality/IComparable contracts (the former having been marked as obsolete).

• [x] re-introduced the QuantityValue with all standard numeric operations (and implicit conversion from double/decimal)
• [x] updated the CodeGen replacing all occurrences of the double with QuantityValue
• [x] restored the IEquality contract using the QuantityValue (which is no longer prone to rounding errors)
• [x] implemented a custom expression analyzer in order to help pre-process (and simplify) the expressions defined in the json definitions
• [x] ensure that the tests are passing with the existing tolerances
• [ ] allow the use of NaN / Infinity (see https://github.com/danm-de/Fractions/issues/26)
• [ ] fix the required [DataContract] / [DataMember] annotations and see about the backwards-compatibility
• [ ] implement lossless versions of the JsonConverters (the AbbreviatedUnitsConverter currently uses the "G36" which should be fine for most cases, but not ideal for values such as "1/3")
• [ ] replace the non-invertible units (relying on irrational functions) with properties (with optional precision): see Pressure.MetersOfElevation and Angle.Tilt
• [ ] see about avoiding the transition from log-space for the conversion operations of the quantities such as the AmplitudeRatio
• [ ] replace all abstract values generated for the tests with QuantityValue (the implicit conversion from double should work for most cases, but we would probably want the full precision of the QuantityValue for some)
• [ ] remove all Tolerances from the tests
• [ ] (optional) introduce a separate package: ("something like") UnitsNet.Light - which is generated based of the "precise/exact" version (without duplicating the custom code/tests)

Breaking changes:

• IQuantiy.Value changes from double to QuantityValue (breaks the IHaveQuantity)
• The conversion from QuantityValue to double is now explicit (unless we're talking about the TypeConverter stuff, I think that still works)
• Might not be possible to maintain compatibility with the old DataContract schema (affects the default DataContract XML / JSON serailizers, for which we cannot have a "Converter/Surrogates" since .NET Core)

[lipchev]

@angularsen @tmilnthorp @Muximize Hey guys, don't mean to push, but It would be helpful if we could have a discussion about the changes proposed in this PR. Specifically, if we assume that the Fractions-based implementation proposed here is a working POC (which I believe it is)- then how would we want to define the public constructors/members in the code-base (this decision is what stopping me from moving on to fixing the tests etc).

Here's a quick "showcase" of the proposed features: (sorry, for some reason the format isn't preserved when I try to copy from it) https://github.com/angularsen/UnitsNet/issues/1367#issuecomment-1979595326

The breaking changes in what I've offered here (which is the version with the "maximum number of breaking changes") stem from the following observations:

• There are only explicit constructors from double and decimal to Fraction (You can implicitly cast int, uint, long, ulong or BigInteger to Fraction)
• You can explicitly cast from Fraction to any supported data type (int, uint, long, ulong, BigInteger, decimal, double). However, be aware that an OverflowException will be thrown, if the target data type's boundary values are exceeded.

And here are 3 options that I see (apply to both constructors and members):

1. Max pain: what I've done here- every double is purged from the code-base, replaced by the Fraction type: user needs to write var mass = Mass.FromGrams(Fraction.FromDouble(0.1))
2. Medium pain: we duplicate the constructors (preserving the ones from double) and only change the IQuantity.Value / As(unit) and the Properties to Fraction (at least we're not dealing with any possible overflows): user needs to write: double value = (double)mass.Value
3. Little pain: we keep the QuantityValue and hide the Fraction inside it (purging all occurrences of the Fraction class from all public constructors/members): the user won't be able to count on Mass.FromGrams(someMass.Grams) == someMass

And obviously there is everything in between. Here's a reminder of some of the items we need to decide on:

1. The constructors
2. The static FromXXX builders
3. The Value
4. The As(unit)
5. The conversion properties (e.g. mass.Grams)

I have no problem with either options (not even sure which version I'd vote for, if there was such a vote..).

[angularsen]

@lipchev I'll need a bit of time to get to this, will try to in the next couple of days.

On the surface, it seems to me Little pain is the only viable path among those 3 options.

[Muximize]

But if the Little pain option means Mass.FromGrams(someMass.Grams) == someMass won't work, doesn't that defeat the purpose of changing the underlying type?

[lipchev]

But if the Little pain option means Mass.FromGrams(someMass.Grams) == someMass won't work, doesn't that defeat the purpose of changing the underlying type?

Yeah, sorry I wasn't clear: what I meant to say is that if you convert to a double (I failed to notice that there wouldn't actually be a conversion to double in this scenario) and then try to construct back the value from double, the equality might break (I was trying to express my general fear of having an implicit conversion with double).

[angularsen]

I still intend to get to this, but I have a hard time finding the time to dive deep.

I have not yet reviewed and confirmed the benchmarks as much as I'd need to, but my gut tells me that to avoid upsetting a lot of people, we can't introduce significant performance hits or change the syntax in any significant way, unless it's totally worth it.

I was thinking, how about we roll a separate nuget for this? Something like UnitsNet.Xp.Fractional.

It would be a competing alternative to UnitsNet and very explicitly described as experimental to test it out and gather feedback. I feel that messing with the fundamentals of the package has too big of a risk and requires much thought and feedback, and I just really struggle to find the time and energy to be a good steward for this project and move heavier discussions forward.

An experimental package could just YOLO a bit, no reviews needed. I'd be more comfortable making changes later, if such a package gained traction and interest. Not too different from the NanoFramework nugets, and previously the Windows Runtime Components nugets.

The same approach could be done for other ancient ideas too, like moving from struct to class and use inheritance, or go all in on generics to support any number type.

The only downside I see besides the one time effort of setting up the packages and codegen, is that it's not so easy for consumers that have transitive/nested dependencies on UnitsNet to try it out.

What do you think?

[lipchev]

I fear this would require a ton of code duplication (code-gen, tests, serialization and all custom code related to the affected interfaces) as well as the increased contribution effort of having two tests to implement. The worst part is that if we roll out v6 with the QuantityValue gone- we'd be forced to split up the core interfaces, no matter which option we choose for the for the Fractions...

I suggest we try to implement this as the "Little pain" variant- keeping up to breaking changes (from v5->v6) to a minimum and roll that out for testing (we could post some more detailed benchmarks here beforehand)..

[lipchev]

Alternatively, we could make yet another "experimental" branch (e.g. v6-fractions) - but we still have to pick one of the proposed options to implement :)

[lipchev]

@angularsen I think the code is almost good- there a few more internals to optimize, but as far as the public API I think this cover all changes.

I've updated everything using the QuantityValue (which I've resurrected and extended), there are currently no public references to the Fraction type anywhere. I'll try to push whatever extensions might be useful to the original library, but generally speaking- we are only a few methods away from having the whole of the Fraction class implemented in the QuantityValue (at which point it would probably make sense for it to go into a separate project).

I've updated the PR to reflect the direction that I think this is heading into..

If you get the time, I this might be a good moment to pull-down my branch for a spin, and double-check that we are regression-free thus far (with the Tolerances still active). Currently there are only a handful of changes to the CustomCode/Tests so it should be relatively easy to review.

In the meanwhile, I'm going to see about fixing the open issues regarding the DataContract / NaN and the log-arithmetic which (I hope) are the only pain-points remaining..

PS Here are some numbers to look at (we should expect about the same from our own benchmarks).

[angularsen]

Sorry this is taking so long on my end, it's just crazy busy with work and family. I intend to get to this.

[lipchev]

Sorry this is taking so long on my end, it's just crazy busy with work and family. I intend to get to this.

Please note- I must have clicked the "synchronize" button at some point, which has merged the changes from the main branch.. 😊 Anyway, I've already prepared a cleaner version- where all tests are currently green, I'll post a new PR as soon as Fractions v8 is published.

[Muximize]

I did not go through all of this thoroughly so maybe I'm missing something obvious, but I was wondering how this would compare to just replacing double wholesale with decimal. Would that (partially) solve the rounding/Equality/IComparable issues? I guess it would be favorable in terms of performance and memory usage, and not introducing an external dependency.

[lipchev]

I did not go through all of this thoroughly so maybe I'm missing something obvious, but I was wondering how this would compare to just replacing double wholesale with decimal. Would that (partially) solve the rounding/Equality/IComparable issues? I guess it would be favorable in terms of performance and memory usage, and not introducing an external dependency.

Assuming we're talking about a decimal that isn't constrained by size/precision - there would still be the matter of the non-terminating decimals such as the result of 1.0 / 3.0. So imagine this operation:

var oneThirdTheValue = value / 3;
var oneThirdTheValueTimesThree = oneThirdTheValue * 3;
Debug.Assert(oneThirdTheValueTimesThree == value)