QuantityValue implemented as a fractional number 🐲

[lipchev]

• QuantityValue implemented as a fractional number
• IQuantity interfaces optimized (some methods refactored as extensions)
• UnitInfo: introduced two new properties: ConversionFromBase and ConversionToBase which are used instead of the switch(Unit) conversion
• UnitsNetSetup: introduced helper methods for adding external quantities, or re-configuring one or more of the existing ones
• UntAbbreviationsCache: introduced additional factory methods (using a configuration delegate)
• UnitParser: introduced additional factory methods (using a configuration delegate)
• UnitConverter: re-implemented (multiple versions)
• Inverse relationship mapping implemented as a type of implicit conversion
• updated the JsonNet converters
• introducing the SystemTextJson project
• added a new UnitsNetConfiguration project to the Samples, showcasing the new configuration options
• many more tests and benchmarks (perhaps too many)

[lipchev]

@angularsen Clearly, I don't expect this to get merged in the Gitty up! fashion, but at least we have the whole picture, with sources that I can reference.

If you want, send me an e-mail, we could do a quick walk-through / discussion.

[angularsen]

100k lines removed, 100k lines added 🙈

[lipchev]

100k lines removed, 100k lines added 🙈

I tried to create this PR twice before (many months ago), while the changes to the unit definitions were still not merged- and the web interface was giving me an error when trying to browse the files changed.. Something like "Too many files to display" 😄

[angularsen]

Ok, I'm not going to get through a review of this many files anytime soon.
Maybe we should have a screen sharing session and go through it together. I may have some time this weekend, what about you? What timezone are you in?

On the surface though, it seems like this could be split up into chunks. I know it's tedious and extra work, but it will be way faster to review. Do you see any chunks of changes to easily split off into separate PRs?

[lipchev]

Ok, I'm not going to get through a review of this many files anytime soon. Maybe we should have a screen sharing session and go through it together. I may have some time this weekend, what about you? What timezone are you in?

Sofia (GMT+3), but time zones are not relevant to my sleep schedule - so basically any time you want.

On the surface though, it seems like this could be split up into chunks. I know it's tedious and extra work, but it will be way faster to review. Do you see any chunks of changes to easily split off into separate PRs?

Yes, I do have some ideas:

1. UnitAbbreviationsCache and the UnitParser should be more or less free of changes once UnitAbbreviationsCache.CreateEmpty should use the default QuantityInfoLookup #1548 is merged
2. I plan to remove the IConvertible interface tonight (lots of red points there)
3. The QuantityParser has just a few minor changes which I was going to try to push as well (other than that it's mostly just double changing to QuantityValue)
4. QuantityFormatter - there was an issue that I created earlier that should (mostly) solve the differences
5. Refactoring the QuantityInfo can theoretically be done without the ConversionExpressions (which would open the way for the changes to the IQuantity interface and some of the extension methods).
6. UnitParser: introduce two new method: GetUnitFromAbbreviation and TryGetUnitFromAbbreviation returning a UnitInfo (which could be used for constructing an instance of the quantity)
7. Introduce the changes to the IQuantity interface and some of the extension methods
8. Move the exceptions in their own folder and replace the usages of the NotImplementedException with the appropriate UnitNotFoundException
9. Update the UnitTestBaseClassGenerator - I've refactored the Parse/TryParse tests (completing the test coverage) - having a look at the diff on the MassTestsBase.g.cs, it looks like these account for about half of all diffs in the PR 😄
10. Make the QuantityInfoLookup public: apart from the extra constructors, there doesn't appear to be any other differences- and I don't see any reason to keep it internal
11. Introduce the CodeGen/Helpers/ExpressionEvaluationHelpers.cs + CodeGen/Helpers/ExpressionAnalyzer and replace the unit conversion expressions such as (_value / 1e3) * 1e-2d) with the simplified expression (unless we actually plan to use the the rest of this PR- this would probably be an overkill).
12. The JsonQuantityConverter stuff from SystemText could theoretically come with it's double versions first (but we do need to have a discussion about it)

Hopefully by the time we get to 5) you'd be up to speed (and fed up with PRs) and we can turn back to reviewing / working on the rest of it as a whole 😄

[angularsen]

Ok, sounds good. Just send PRs my way and I'll try to get to them. I have a little bit of extra time this weekend.

[github-actions]

This PR is stale because it has been open 60 days with no activity. Remove stale label or comment or this will be closed in 7 days.

[github-actions]

This PR was automatically closed due to inactivity.

[lipchev]

@angularsen I'm sorry, but P0 is a no-go. I have been very explicit about it since the very beginning (even before this PR).
You could try changing it (locally), just to get an idea of how many things would break (ambiguous assignments).

[angularsen]

I see, I'll try and change it locally to get a better feel for why adding implicit cast is challenging.
The way I understand this breaking change and how it will affect consumers' code, it currently feels too invasive of a change, but I'll need to map out those use cases to know for sure.

[angularsen]

Here is a follow up, trying to map out the two alternatives to avoid or reduce the breaking change of consumer code, while retaining the value of this PR.

I had Claude implement each one and evaluate the impact on internal code and tests, as well as consunmer code.

As far as I can tell, option A with bi-directional implicit cast of QV and double is very much feasible?

PR 1544 Review: QuantityValue as Fractional Number

The architectural improvements (ConversionExpression, centralized UnitConverter, builder pattern, exact rational coefficients) are solid and we want to get this merged.

The main concern is the breaking change impact of replacing double with QuantityValue across the public API. We evaluated two approaches to mitigate this and implemented both to get empirical data.

---

The problem

Without mitigation, ~60-70% of consumer code breaks on upgrade:

double meters = length.Meters;          // ❌ won't compile (returns QuantityValue)
double ratio = len1 / len2;             // ❌ won't compile (returns QuantityValue)
Math.Round(length.Meters, 2);           // ❌ won't compile
SomeApi(length.Value);                  // ❌ won't compile

---

Option A: Add implicit conversion QuantityValue → double (recommended)

Change one keyword in QuantityValue.ConvertToNumber.cs:

// Before:
public static explicit operator double(QuantityValue value) => value.ToDouble();
// After:
public static implicit operator double(QuantityValue value) => value.ToDouble();

Why it works

The PR is clean — no generated operator takes double. Every operator uses QuantityValue exclusively:

Length operator *(QuantityValue left, Length right)
Length operator *(Length left, QuantityValue right)
Length operator /(Length left, QuantityValue right)
QuantityValue operator /(Length left, Length right)

With bidirectional implicit conversions (double ↔ QuantityValue), C# overload resolution prefers user-defined operators over built-in ones when both require one implicit conversion. So QV + double resolves to user-defined QV + QV, not built-in double + double.

Verified empirically (branch agl/pr-1544-review-option-a-implicit-cast)

9d734de

We implemented this and built/tested the full solution:

• Library code: 1 line changed. Zero compilation errors.
• Test code: 101 Assert.Equal(double, QuantityValue) calls needed (double) casts to resolve xUnit overload ambiguity — a mechanical fix, not a behavioral change.
• Build: 0 errors, 0 warnings.
• Tests: 0 new failures across ~51,900 tests.

The xUnit ambiguity is specific to Assert.Equal's many overloads (including DateTime). Standard consumer code — method calls, assignments, operators, comparisons — is unaffected.

Consumer code scenarios

Scenario	Works?	Explanation
double meters = length.Meters	✅	Implicit conversion
double ratio = len1 / len2	✅	Operator returns QV, implicit converts
Math.Round(length.Meters, 2)	✅	Resolves to Math.Round(double, int)
SomeDoubleApi(length.Meters)	✅	Implicit conversion
Length * double	✅	double promotes to QV, uses Length * QV
double x = double + QV	✅	Operator returns QV, implicit converts for assignment
var x = length.Meters	✅	x is QV — implicitly converts wherever double is expected

Trade-offs

• var x = length.Meters infers QuantityValue — consumers see this in IDE tooltips. Not a functional issue (implicit conversion handles everything), but visible.
• Precision loss from QuantityValue → double is silent. This is the desired trade-off for most consumers. Those who want exact precision can stay on QuantityValue directly.
• Bidirectional implicit conversion is unusual in C#, but our empirical testing confirms it works cleanly here.

---

Option B: Keep double public, use QuantityValue internally (not recommended)

Revert the public API to double while keeping QuantityValue for the internal conversion pipeline. We fully implemented this to evaluate the trade-offs.

Verified empirically (branch agl/pr-1544-review-option-b-double-public)

37d1641

• 608 files changed: ~10 manual source files, CodeGen templates (~15 locations), UnitRelations.json, all 131 generated quantities regenerated, serialization adapters, test and benchmark code.
• Build: 0 library errors, 0 warnings.
• Tests: 2,509 failures out of 51,899 (5%).

The failures are not test bugs

They reveal a fundamental precision loss from double storage:

Failure category	Count	Root cause
ToUnit_FromNonBaseUnit	1,215	Roundtrip conversion loses precision — double introduces rounding at each step
ToUnit_FromIQuantity	491	Same root cause
ToString	256	Precision-sensitive formatting
ConversionRoundTrip	68	Direct roundtrip tests

With QuantityValue, converting 3.0 feet → meters → feet is lossless (exact rational arithmetic). With double storage, each conversion step introduces floating-point rounding that compounds. This defeats a core value proposition of the PR.

---

Comparison

	Option A	Option B
Library code changes	1 line	608 files
Build errors	0	0
New test failures	0	2,509 (5%)
Precision preserved	Full	Degraded (roundtrip loss)
Consumer breaking changes eliminated	~60-70%	~60-70%
Future flexibility	Consumers can use QuantityValue directly	Requires later API change

Recommendation

Option A. One line of library code, zero new test failures, full precision preserved. It eliminates the largest consumer-facing breaking change while keeping 100% of the PR's value.

---

Other items to address before merge

Must do

1. Revert EmitDefaultValue = false on DataMember — Length.Zero serialized via DataContract should always include both Value and Unit. @angularsen flagged this.

Should do

2. Document DataContract serialization as a breaking change in v6 release notes. The _value field type change means existing DataContract-serialized data won't deserialize. XML surrogate works; JSON surrogate blocked by .NET runtime bug. Recommend migrating to JsonNet or System.Text.Json packages.

Post-merge

3. Migration guide for this PR's breaking changes (to be expanded for full v6 later).
4. Test coverage for InterfaceQuantityWithUnitTypeConverter — appears unused/untested.
5. Align QuantityValueFormatOptions enum names — DecimalPrecision vs ExactNumber should use consistent naming.
6. Configure extension on UnitDefinition[] — consider wrapper type or static method for cleaner API.

Remaining breaking changes (regardless of QuantityValue approach)

Change	v5	v6
As(), ToUnit()	Interface methods	Extension methods (some [Obsolete])
UnitConverter() constructor	Public, parameterless	Removed
SetConversionFunction / GetConversionFunction	Available	Removed
UnitsNetSetup constructor	Public	Private; use builder
DataContract serialization	double field	QuantityValue struct
AbbreviatedUnitsConverter (JsonNet)	IReadOnlyDictionary ctor	UnitParser + options
Default JSON precision	~17 digits	Up to 29 digits
MissingMemberHandling.Error	Silently skipped unknowns	Correctly throws (bug fix)
Null IQuantity deserialization	Returns .Zero	Returns null

[github-actions]

This PR is stale because it has been open 60 days with no activity. Remove stale label or comment or this will be closed in 7 days.

[github-actions]

This PR was automatically closed due to inactivity.

[lipchev]

@angularsen I've reopened this:

1. I've sync the changes from upstream (see 7d1f055) - I kept the obsolete usages, as to avoid the performance overhead of the dictionary lookup. I'm not too sure about the extension methods, and the usages of the default configuration - we can make the IQuantity.QuantityInfo property internal, or we could force the converter to do the dictionary lookup. However we should note that if we were to completely remove the IQuantity.QuantityInfo property from the #NET build, then using the GetQuantityInfo extension would fail for HowMuch, unless it is registered in the default configuration.
2. I've made the QuantityValue -> double operator implicit. I tested the package with my own projects and frankly there were significantly more line changes that I had to make in order to get everything to compile, compared to the actual migration of IQuantity.Value to QuantityValue 😆 I don't think this would be the case for most people, but there were a lot of Assert.Equals that failed for me too...
3. Finally, I removed the EmitDefaultValue = false from the quantity unit and value