From db9003e6a9d0f7ec3554ee16c27b8852f90534c6 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 30 Sep 2026 01:27:15 +0000 Subject: [PATCH 1/2] Convert temperature differences with the factor alone, without the scale offset [patch] MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit TemperatureDelta, TemperatureRise and TemperatureDrop hold a difference of temperatures, but their From{Unit} factories added the Celsius/Fahrenheit offset and In(unit) subtracted it through FromBase. A 10 °C rise was stored as 283.15 K, and a 10 K rise read back as -263.15 °C. The generator now leaves the offset out of every V1 scalar factory, and emits a factor-only In(unit) for V1 forms of a dimension that has an offset unit. Absolute temperatures (V0) are unchanged. Behaviour change for callers of the three difference types' Celsius and Fahrenheit conversions. Fixes #283 Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_015i2yeKwKRWtHhdc6NPySjW --- CLAUDE.md | 2 +- .../TemperatureDelta.g.cs | 6 +-- .../TemperatureDrop.g.cs | 6 +-- .../TemperatureRise.g.cs | 6 +-- .../Generators/QuantitiesGenerator.cs | 53 ++++++++++++++----- .../Quantities/StorageConversionTests.cs | 21 ++++++++ docs/physics-generator.md | 2 +- 7 files changed, 72 insertions(+), 24 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 0a07ee7..69a8af7 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -499,7 +499,7 @@ var converted = sourceString.As(); - **SEM007** — a metadata file could not be parsed. Replaces the base generator's `CONV001` in category `SourceGenerator`, and covers the path that used to swallow the exception, where a malformed `units.json` silently produced factories with no scale factor. - **SEM008** — a relationship's declared result does not follow from the dimensions of its operands, or its value is signed and the declared result is a magnitude. The check comes from `Semantics.Vocabulary`, shared with the C++ projection; before that this side checked the names (SEM001) and the forms (SEM003) and then emitted the operator, so `Sensitivity * Pressure -> ElectricPotential` shipped as a working C# operator computing the wrong physics — which is what found that bug, and it is now fixed. **No operator is generated** for a refused relationship, in any of the directions C# spells a product in — that followed from making the vocabulary drive emission rather than only check it, and the removal is documented in `docs/migration-guide-5.0.md`. Suppressed in `Semantics.Quantities.csproj` because ktsu.Sdk builds warnings as errors and the four below are outstanding; `UnkeepableRelationshipTests` pins the set, and asserts that none of them is in the compiled surface, so a fifth fails there rather than disappearing into the suppression. - **SEM009**: a factor's `value` in `conversions.json` is neither a decimal literal nor a fraction of two with a non-zero denominator, or a `double` cannot hold it (a literal, operand, or quotient beyond its range, or a non-zero value that rounds to zero). An error, and no constant is generated for it, because every unit using the factor would otherwise fail to compile far from the metadata line that caused it, or convert with a wrong factor. - - **SEM010**: a dimension declares both a vector form and a unit converting with an additive offset. Adding 273.15 to each component of a displacement is not a unit change, so the whole per-unit surface — every `From{Unit}` factory and the `In(unit)` reader — is withheld from that dimension's vector types rather than emitted quietly wrong. Withheld as a whole rather than per-unit, because `In` takes the dimension's `I{Dimension}Unit` and would accept the offset unit at runtime even if only its factory were skipped. The scalar forms are unaffected: the offset is correct for a V0 or V1. Defensive — no dimension declaring a vector form has an offset unit today, and `TheRealMetadataReportsNothingUnexpected` is what keeps that true. + - **SEM010**: a dimension declares both a vector form and a unit converting with an additive offset. Adding 273.15 to each component of a displacement is not a unit change, so the whole per-unit surface — every `From{Unit}` factory and the `In(unit)` reader — is withheld from that dimension's vector types rather than emitted quietly wrong. Withheld as a whole rather than per-unit, because `In` takes the dimension's `I{Dimension}Unit` and would accept the offset unit at runtime even if only its factory were skipped. The scalar forms are unaffected: a V0 applies the offset, and a V1 (a difference, such as `TemperatureDelta`) converts with the factor alone in both `From{Unit}` and `In(unit)` (#283). Defensive — no dimension declaring a vector form has an offset unit today, and `TheRealMetadataReportsNothingUnexpected` is what keeps that true. - Descriptors are allocated from `SemanticsDiagnostics`, which is the one place to add a new one. `AnalyzerReleaseTrackingTests` fails if the identifier is missing from `AnalyzerReleases.Unshipped.md`, so RS2008 no longer surfaces only after a push. - See `docs/physics-generator.md` for the full schema and an end-to-end "add a dimension" walk-through. diff --git a/Semantics.Quantities/Generated/Semantics.SourceGenerators/Semantics.SourceGenerators.QuantitiesGenerator/TemperatureDelta.g.cs b/Semantics.Quantities/Generated/Semantics.SourceGenerators/Semantics.SourceGenerators.QuantitiesGenerator/TemperatureDelta.g.cs index da6a766..1c10e0d 100644 --- a/Semantics.Quantities/Generated/Semantics.SourceGenerators/Semantics.SourceGenerators.QuantitiesGenerator/TemperatureDelta.g.cs +++ b/Semantics.Quantities/Generated/Semantics.SourceGenerators/Semantics.SourceGenerators.QuantitiesGenerator/TemperatureDelta.g.cs @@ -109,14 +109,14 @@ namespace ktsu.Semantics.Quantities; /// /// The value in Celsius. /// A new instance. - public static TemperatureDelta FromCelsius(T value) => Create((value + Units.ConversionConstants.Values.CelsiusToKelvinOffset)); + public static TemperatureDelta FromCelsius(T value) => Create(value); /// /// Creates a new from a value in Fahrenheit. /// /// The value in Fahrenheit. /// A new instance. - public static TemperatureDelta FromFahrenheit(T value) => Create(((value * Units.ConversionConstants.Values.FahrenheitScale) + Units.ConversionConstants.Values.FahrenheitToKelvinOffset)); + public static TemperatureDelta FromFahrenheit(T value) => Create((value * Units.ConversionConstants.Values.FahrenheitScale)); /// /// Creates a new from a value in Rankine. @@ -131,7 +131,7 @@ namespace ktsu.Semantics.Quantities; /// /// The dimensionally-compatible target unit. /// The value expressed in . - public T In(global::ktsu.Semantics.Quantities.ITemperatureUnit unit) => unit.FromBase(Value); + public T In(global::ktsu.Semantics.Quantities.ITemperatureUnit unit) => Value / unit.ToBaseFactorAs(); /// /// Gets the magnitude of this quantity as a . diff --git a/Semantics.Quantities/Generated/Semantics.SourceGenerators/Semantics.SourceGenerators.QuantitiesGenerator/TemperatureDrop.g.cs b/Semantics.Quantities/Generated/Semantics.SourceGenerators/Semantics.SourceGenerators.QuantitiesGenerator/TemperatureDrop.g.cs index cc8a59c..12bba0e 100644 --- a/Semantics.Quantities/Generated/Semantics.SourceGenerators/Semantics.SourceGenerators.QuantitiesGenerator/TemperatureDrop.g.cs +++ b/Semantics.Quantities/Generated/Semantics.SourceGenerators/Semantics.SourceGenerators.QuantitiesGenerator/TemperatureDrop.g.cs @@ -110,14 +110,14 @@ namespace ktsu.Semantics.Quantities; /// /// The value in Celsius. /// A new TemperatureDrop instance. - public static TemperatureDrop FromCelsius(T value) => Create((value + Units.ConversionConstants.Values.CelsiusToKelvinOffset)); + public static TemperatureDrop FromCelsius(T value) => Create(value); /// /// Creates a new TemperatureDrop from a value in Fahrenheit. /// /// The value in Fahrenheit. /// A new TemperatureDrop instance. - public static TemperatureDrop FromFahrenheit(T value) => Create(((value * Units.ConversionConstants.Values.FahrenheitScale) + Units.ConversionConstants.Values.FahrenheitToKelvinOffset)); + public static TemperatureDrop FromFahrenheit(T value) => Create((value * Units.ConversionConstants.Values.FahrenheitScale)); /// /// Creates a new TemperatureDrop from a value in Rankine. @@ -132,7 +132,7 @@ namespace ktsu.Semantics.Quantities; /// /// The dimensionally-compatible target unit. /// The value expressed in . - public T In(global::ktsu.Semantics.Quantities.ITemperatureUnit unit) => unit.FromBase(Value); + public T In(global::ktsu.Semantics.Quantities.ITemperatureUnit unit) => Value / unit.ToBaseFactorAs(); /// Implicit conversion to TemperatureDelta. public static implicit operator TemperatureDelta(TemperatureDrop value) => TemperatureDelta.Create(value.Value); diff --git a/Semantics.Quantities/Generated/Semantics.SourceGenerators/Semantics.SourceGenerators.QuantitiesGenerator/TemperatureRise.g.cs b/Semantics.Quantities/Generated/Semantics.SourceGenerators/Semantics.SourceGenerators.QuantitiesGenerator/TemperatureRise.g.cs index 6df1151..9c3f975 100644 --- a/Semantics.Quantities/Generated/Semantics.SourceGenerators/Semantics.SourceGenerators.QuantitiesGenerator/TemperatureRise.g.cs +++ b/Semantics.Quantities/Generated/Semantics.SourceGenerators/Semantics.SourceGenerators.QuantitiesGenerator/TemperatureRise.g.cs @@ -110,14 +110,14 @@ namespace ktsu.Semantics.Quantities; /// /// The value in Celsius. /// A new TemperatureRise instance. - public static TemperatureRise FromCelsius(T value) => Create((value + Units.ConversionConstants.Values.CelsiusToKelvinOffset)); + public static TemperatureRise FromCelsius(T value) => Create(value); /// /// Creates a new TemperatureRise from a value in Fahrenheit. /// /// The value in Fahrenheit. /// A new TemperatureRise instance. - public static TemperatureRise FromFahrenheit(T value) => Create(((value * Units.ConversionConstants.Values.FahrenheitScale) + Units.ConversionConstants.Values.FahrenheitToKelvinOffset)); + public static TemperatureRise FromFahrenheit(T value) => Create((value * Units.ConversionConstants.Values.FahrenheitScale)); /// /// Creates a new TemperatureRise from a value in Rankine. @@ -132,7 +132,7 @@ namespace ktsu.Semantics.Quantities; /// /// The dimensionally-compatible target unit. /// The value expressed in . - public T In(global::ktsu.Semantics.Quantities.ITemperatureUnit unit) => unit.FromBase(Value); + public T In(global::ktsu.Semantics.Quantities.ITemperatureUnit unit) => Value / unit.ToBaseFactorAs(); /// Implicit conversion to TemperatureDelta. public static implicit operator TemperatureDelta(TemperatureRise value) => TemperatureDelta.Create(value.Value); diff --git a/Semantics.SourceGenerators/Generators/QuantitiesGenerator.cs b/Semantics.SourceGenerators/Generators/QuantitiesGenerator.cs index 9209f06..7d73f69 100644 --- a/Semantics.SourceGenerators/Generators/QuantitiesGenerator.cs +++ b/Semantics.SourceGenerators/Generators/QuantitiesGenerator.cs @@ -574,6 +574,9 @@ private static void ReportUnknownUnitReferences( /// physicalConstraints.minExclusive: "0" per #51) the guard is upgraded to /// Vector0Guards.EnsurePositive, which rejects zero as well as negative values. /// is ignored when is false. + /// When is true (a V1 form, which holds a signed difference + /// rather than a position on the scale) the unit's offset is left out, so + /// TemperatureDelta.FromCelsius(10) is 10 K rather than 283.15 K (#283). /// private static void AddUnitFactories( ClassTemplate cls, @@ -582,7 +585,8 @@ private static void AddUnitFactories( string fullType, string crefForComment, bool applyV0Guard, - bool strictPositive = false) + bool strictPositive = false, + bool isDifference = false) { if (availableUnits == null || availableUnits.Count == 0) { @@ -597,7 +601,7 @@ private static void AddUnitFactories( bool isBase = unitName == baseUnit; string conversionExpr = isBase ? Emit.ValueParameter - : BuildToBaseExpression(unitName, unitMap, Emit.ValueParameter); + : BuildToBaseExpression(unitName, unitMap, Emit.ValueParameter, applyOffset: !isDifference); string body = applyV0Guard ? $"=> Create(Vector0Guards.{guardMethod}({conversionExpr}, nameof(value)));" @@ -651,10 +655,15 @@ private static void AddUnitFactories( /// vector factories (#237) pass one component parameter at a time, which is the only reason /// this is a parameter rather than the constant it used to be. /// + /// + /// Whether to add the unit's offset. False for a form that holds a difference (#283): 10 °C + /// warmer is 10 K warmer, not 283.15 K. + /// private static string BuildToBaseExpression( string unitName, IReadOnlyDictionary unitMap, - string operand) + string operand, + bool applyOffset = true) { // If we don't have unit metadata, fall back to identity. The dimensions.json author is // responsible for keeping availableUnits in sync with units.json; if a unit is missing, @@ -684,7 +693,7 @@ private static string BuildToBaseExpression( scaled = $"({operand} * Units.ConversionConstants.Values.{unit.ConversionFactor})"; } - if (HasOffset(unit)) + if (applyOffset && HasOffset(unit)) { scaled = $"({scaled} + Units.ConversionConstants.Values.{unit.Offset})"; } @@ -711,8 +720,24 @@ private static bool HasOffset(UnitDefinition? unit) => /// caller's unit. Emitted for V0 and V1 (scalar-storage) types only; vector V2+ types /// have per-component conversion needs and are deferred. /// - private static void AddDimensionAndInMembers(ClassTemplate cls, PhysicalDimension dim) + /// + /// A difference form () of a dimension with an offset unit reads + /// with the factor alone, the counterpart of its factories leaving the offset out (#283). + /// unit.FromBase would subtract the offset, reading a 10 K rise as −263.15 °C. Every other + /// form keeps FromBase, which for a unit without an offset is the same division. + /// + private static void AddDimensionAndInMembers( + ClassTemplate cls, + PhysicalDimension dim, + IReadOnlyDictionary unitMap, + bool isDifference) { + bool factorOnly = isDifference + && dim.AvailableUnits.Any(u => unitMap.TryGetValue(u, out UnitDefinition? d) && HasOffset(d)); + string inBody = factorOnly + ? "=> Value / unit.ToBaseFactorAs();" + : "=> unit.FromBase(Value);"; + cls.Members.Add(new FieldTemplate() { Comments = {$"/// Gets the physical dimension this quantity belongs to."}, @@ -737,7 +762,7 @@ private static void AddDimensionAndInMembers(ClassTemplate cls, PhysicalDimensio { new ParameterTemplate { Type = $"global::ktsu.Semantics.Quantities.I{dim.Name}Unit", Name = "unit" }, }, - BodyFactory = (body) => body.Write("=> unit.FromBase(Value);"), + BodyFactory = (body) => body.Write(inBody), }); } @@ -1005,7 +1030,7 @@ private void EmitV0BaseType( applyV0Guard: true); // Dimension override + typed In() (#59). - AddDimensionAndInMembers(cls, dim); + AddDimensionAndInMembers(cls, dim, emission.Units, isDifference: false); // V0 - V0 returns the same V0 of T.Abs(left - right) (locked decision in #52). // We emit this on every V0 base type so the derived operator wins overload resolution @@ -1089,17 +1114,18 @@ private void EmitV1BaseType( }); // Factory methods for every available unit. - // V1 quantities are signed; no V0 non-negativity guard. + // V1 quantities are signed differences: no V0 non-negativity guard, and no unit offset (#283). AddUnitFactories( cls, dim.AvailableUnits, emission.Units, fullType, "", - applyV0Guard: false); + applyV0Guard: false, + isDifference: true); // Dimension override + typed In() (#59). - AddDimensionAndInMembers(cls, dim); + AddDimensionAndInMembers(cls, dim, emission.Units, isDifference: true); // Magnitude method returning V0 base cls.Members.Add(new MethodTemplate() @@ -1259,7 +1285,7 @@ private void EmitOverloadType( // type (#50). V0 overloads that declare physicalConstraints.minExclusive in // dimensions.json (#51, e.g. Wavelength, Period, HalfLife) get the stricter // EnsurePositive guard so a zero input is rejected too. V1 overloads accept - // any sign. + // any sign, and like their V1 base hold a difference, so they leave out the unit offset (#283). bool strictPositive = type.Magnitude == Magnitude.Positive; AddUnitFactories( cls, @@ -1268,10 +1294,11 @@ private void EmitOverloadType( fullType, typeName, applyV0Guard: vectorForm == 0, - strictPositive: strictPositive); + strictPositive: strictPositive, + isDifference: vectorForm != 0); // Dimension override + typed In() (#59). - AddDimensionAndInMembers(cls, dim); + AddDimensionAndInMembers(cls, dim, emission.Units, isDifference: vectorForm != 0); // Implicit widening to base type cls.Members.Add(new MethodTemplate() diff --git a/Semantics.Test/Quantities/StorageConversionTests.cs b/Semantics.Test/Quantities/StorageConversionTests.cs index 16b4bb8..20de7f3 100644 --- a/Semantics.Test/Quantities/StorageConversionTests.cs +++ b/Semantics.Test/Quantities/StorageConversionTests.cs @@ -136,6 +136,27 @@ public void FahrenheitConvertsToKelvinAndCelsius() AssertValue("212", boiling.In(Units.Fahrenheit), terminates: false); } + /// + /// A temperature difference converts with the factor alone (#283): 10 °C warmer is 10 K warmer, + /// not 283.15 K, and a 10 K rise reads as 10 °C rather than −263.15 °C. + /// + [TestMethod] + public void TemperatureDifferencesLeaveOutTheScaleOffset() + { + AssertValue("10", TemperatureDelta.FromCelsius(Of("10")).Value, terminates: true); + AssertValue("10", TemperatureDelta.FromFahrenheit(Of("18")).Value, terminates: false); + AssertValue("10", TemperatureDelta.FromKelvin(Of("10")).In(Units.Celsius), terminates: true); + AssertValue("18", TemperatureDelta.FromKelvin(Of("10")).In(Units.Fahrenheit), terminates: false); + + AssertValue("10", TemperatureRise.FromCelsius(Of("10")).Value, terminates: true); + AssertValue("10", TemperatureRise.FromFahrenheit(Of("18")).Value, terminates: false); + AssertValue("10", TemperatureRise.FromKelvin(Of("10")).In(Units.Celsius), terminates: true); + + AssertValue("10", TemperatureDrop.FromCelsius(Of("10")).Value, terminates: true); + AssertValue("10", TemperatureDrop.FromFahrenheit(Of("18")).Value, terminates: false); + AssertValue("10", TemperatureDrop.FromKelvin(Of("10")).In(Units.Celsius), terminates: true); + } + [TestMethod] public void AnglesUsePiToThePrecisionOfTheStorageType() { diff --git a/docs/physics-generator.md b/docs/physics-generator.md index b097999..3a7ff4b 100644 --- a/docs/physics-generator.md +++ b/docs/physics-generator.md @@ -267,7 +267,7 @@ exist at all is the open question in | SEM007 | A metadata file that could not be parsed. | | SEM008 | A relationship whose declared result does not follow from the dimensions of its operands, or whose signed value cannot land in a magnitude result. No operator is generated for it. | | SEM009 | A `conversions.json` factor whose `value` is neither a decimal literal nor a fraction of two with a non-zero denominator, or that a `double` cannot hold. An error, and no constant is generated for it. | - | SEM010 | A dimension declaring both a vector form and an offset unit. Its vector types get no `From{Unit}` factories and no `In(unit)` reader, because an additive offset applied componentwise is not a unit change. The scalar forms keep theirs, where the offset is correct. | + | SEM010 | A dimension declaring both a vector form and an offset unit. Its vector types get no `From{Unit}` factories and no `In(unit)` reader, because an additive offset applied componentwise is not a unit change. The scalar forms keep theirs: a V0 applies the offset, and a V1 holds a difference, so it converts with the factor alone (#283). | Adding one means adding it to `SemanticsDiagnostics` and to `AnalyzerReleases.Unshipped.md`; `AnalyzerReleaseTrackingTests` fails if the second step is forgotten. `GeneratorDiagnosticTests` proves each one still fires on the input it is meant to catch. - `availableUnits` order matters: the first entry is treated as the SI base unit by `UnitsGenerator`. From a0b52773ed8c48439becfe3c80bdfd0b70cfb08d Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 30 Sep 2026 01:40:40 +0000 Subject: [PATCH 2/2] Derive the difference-form offset rule from applyV0Guard instead of an eighth parameter Addresses SonarCloud S107 on AddUnitFactories. Every call site passed isDifference as the negation of applyV0Guard, so the parameter carried nothing the guard flag did not. Generated output is unchanged. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_015i2yeKwKRWtHhdc6NPySjW --- .../Generators/QuantitiesGenerator.cs | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-) diff --git a/Semantics.SourceGenerators/Generators/QuantitiesGenerator.cs b/Semantics.SourceGenerators/Generators/QuantitiesGenerator.cs index 7d73f69..abb1cf7 100644 --- a/Semantics.SourceGenerators/Generators/QuantitiesGenerator.cs +++ b/Semantics.SourceGenerators/Generators/QuantitiesGenerator.cs @@ -574,8 +574,8 @@ private static void ReportUnknownUnitReferences( /// physicalConstraints.minExclusive: "0" per #51) the guard is upgraded to /// Vector0Guards.EnsurePositive, which rejects zero as well as negative values. /// is ignored when is false. - /// When is true (a V1 form, which holds a signed difference - /// rather than a position on the scale) the unit's offset is left out, so + /// When is false the form is a V1, which holds a signed + /// difference rather than a position on the scale, so the unit's offset is left out: /// TemperatureDelta.FromCelsius(10) is 10 K rather than 283.15 K (#283). /// private static void AddUnitFactories( @@ -585,8 +585,7 @@ private static void AddUnitFactories( string fullType, string crefForComment, bool applyV0Guard, - bool strictPositive = false, - bool isDifference = false) + bool strictPositive = false) { if (availableUnits == null || availableUnits.Count == 0) { @@ -601,7 +600,7 @@ private static void AddUnitFactories( bool isBase = unitName == baseUnit; string conversionExpr = isBase ? Emit.ValueParameter - : BuildToBaseExpression(unitName, unitMap, Emit.ValueParameter, applyOffset: !isDifference); + : BuildToBaseExpression(unitName, unitMap, Emit.ValueParameter, applyOffset: applyV0Guard); string body = applyV0Guard ? $"=> Create(Vector0Guards.{guardMethod}({conversionExpr}, nameof(value)));" @@ -1121,8 +1120,7 @@ private void EmitV1BaseType( emission.Units, fullType, "", - applyV0Guard: false, - isDifference: true); + applyV0Guard: false); // Dimension override + typed In() (#59). AddDimensionAndInMembers(cls, dim, emission.Units, isDifference: true); @@ -1294,8 +1292,7 @@ private void EmitOverloadType( fullType, typeName, applyV0Guard: vectorForm == 0, - strictPositive: strictPositive, - isDifference: vectorForm != 0); + strictPositive: strictPositive); // Dimension override + typed In() (#59). AddDimensionAndInMembers(cls, dim, emission.Units, isDifference: vectorForm != 0);