Report the dimension that has exponents when two claim a unit - #224
Merged
Merged
Conversation
`Radian`, `Degree`, `Gradian`, `Milliradian` and `Revolution` each reported `PhysicalDimensions.Dimensionless`, so an angle measured nothing. That is the conflation the `angle` axis exists to prevent -- without it an angle is the same thing as a ratio -- and the axis was in the metadata the whole time, unreachable through the one property a consumer reads. All five are in `availableUnits` on both `AngularDisplacement` and `Dimensionless`. The marker interfaces carry every claim, so only the singular `Dimension` has to pick one, and `dims[0]` picked by position in the file: `Dimensionless` is its first entry, so a unit it claims could never report anything else. `RadianPerSecond` was right by not being claimed twice. A claim that says something now beats one that says nothing. Where several say something the first still wins, which is right for the only other unit claimed twice: `SquareMeter` is `Area` and `NuclearCrossSection`, one of the collisions the nominal layer exists for, so the two answers differ in name and not in exponents. Nothing is removed -- `Radian` still implements `IDimensionlessUnit`, so `Ratio<T>.FromRadian` and `Ratio<T>.In(new Radian())` compile as before -- and no consumer of `IUnit.Dimension` exists in this repository, since `PhysicalQuantityCore` compares the quantity's dimension rather than the unit's. It has one outside this repository, which is how it was found: `ktsu.Schema` derives the eight exponents in its C++ reflection table from the unit a member names, precisely so the table cannot drift from the unit beside it. A member measured in radians reached C++ as eight zeroes, which is byte-for-byte what a member measuring nothing emits, so a consumer could not tell an angle from a flag. The assembly-wide test is the one that matters: a unit added to both a real dimension and `Dimensionless` tomorrow is the same bug, and naming today's five would not catch it. Fixes #223 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UGHDsYaaTQdzVR4XBR6miu
|
This was referenced Sep 14, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.



Fixes #223.
The bug
Same for
Degree,Gradian,MilliradianandRevolution. An angle measured nothing — the exact conflation theangleaxis exists to prevent, with the axis sitting in the metadata the whole time, unreachable through the one property a consumer reads.All five are in
availableUnitson bothAngularDisplacementandDimensionless. The marker interfaces carry every claim —Radianis emitted asIUnit, IDimensionlessUnit, IAngularDisplacementUnit— so only the singularDimensionproperty has to pick one, anddims[0]picked by position in the file.Dimensionlessis its first entry, so a unit it claims could never report anything else.RadianPerSecondwas right by not being claimed twice.The fix
A claim that says something beats one that says nothing. Where several say something the first still wins, which is right for the only other unit claimed twice:
SquareMeterisAreaandNuclearCrossSection, one of the 72-dimensions-over-63-exponent-vectors collisions the nominal layer exists for, so the two answers differ in name and not in exponents.Option 1 from the issue. Nothing is removed:
Radianstill implementsIDimensionlessUnit, soRatio<T>.FromRadianandRatio<T>.In(new Radian())compile exactly as before. Only the value ofDimensionchanges, and it has no consumer inside this repository —PhysicalQuantityCorecompares the quantity's dimension, not the unit's.Generated diff
Five lines, and nothing else:
SquareMeteris untouched, which is the check that the fallback still fires.Why it was found outside this repository
ktsu.Schema's C++ reflection table carries eight exponents per member and derives them — the member names a unit,UnitRegistryresolves it, the unit'sDimensionInfosupplies the numbers — precisely so the table cannot drift from the unit beside it. A member measured in radians therefore reached C++ as.dimension = { 0, 0, 0, 0, 0, 0, 0, 0 }, byte-for-byte what a member measuring nothing emits. A consumer reading the table could not tell an angle from a flag. ktsu-dev/Schema#179 moves that repository onto a release that can see this.Tests
Two, in a new
UnitDimensionTests. The assembly-wide one is the one that matters: a unit added to both a real dimension andDimensionlesstomorrow is the same bug, and naming today's five would not catch it. It walks the compiled unit types, resolves eachI{Dimension}Unitmarker back to itsDimensionInfo, and asserts that a unit with any exponent-bearing claim reports an exponent-bearing dimension — plus a guard that some unit is claimed twice at all, so the test cannot pass by asserting nothing. The second pins the five angular units toAngularDisplacementand to oneangle.Both fail against the generator as it was and pass against it as it is; I checked by stashing the generator change and re-running.
Verification
dotnet buildover the whole solution (every target framework): 0 warnings, 0 errors.dotnet test -f net10.0: 1165 total, 1157 passed, 8 skipped (Windows-only path tests), 0 failed.dotnet build -p:CustomAfterMicrosoftCommonProps=$PWD/.sonarlint/sonar-local.props: no finding in either file this PR touches. An S3267 on the first draft ofReportedDimensionis why both helpers are written as LINQ; the findings that remain are pre-existing ones inQuantityVocabulary,CppQuantityGeneratorand three test files, untouched here.Units.g.csis in the diff andgit statusis clean after a build. No quantity was added or renamed, so the storage-type alias props are unaffected.CLAUDE.mdanddocs/physics-generator.mdboth record the rule. The CLAUDE.md line saying nothing depends on theangleaxis yet is no longer true and now names what does.🤖 Generated with Claude Code
https://claude.ai/code/session_01UGHDsYaaTQdzVR4XBR6miu
Generated by Claude Code