Pin the logarithmic subtraction edge cases for Level - #1725
Conversation
Level's logarithmic operators are covered by LevelTestsBase for the ordinary case, but not where the linear result stops being positive. Two such cases return silently today: v - v -> -Infinity, since log10(0) is -infinity smaller - larger -> NaN, since log10 of a negative number is undefined The second is inconsistent with the rest of the quantity: the Level(quantity, reference) constructor rejects a non-positive ratio with ArgumentOutOfRangeException and the message "The base-10 logarithm of a number <= 0 is undefined", while operator- performs that same logarithm and yields NaN without signalling anything. These tests pin the current behaviour rather than endorse it, so that any later change to it is a visible decision. Also adds the addition case from angularsen#1569 with the reporter's own values, asserted against 10 * log10(10^0.16 + 10^0.07), since that issue reads the result as wrong on the assumption that adding levels adds their decibel numbers.
|
CI evidence, since this PR shows no checks (first-time-contributor gate). Full matrix run on my fork: https://github.com/Rafael-SOWNet/UnitsNet/actions/runs/31136648228 — success. The same workflow on the same base without this PR's commit passes 52560 in The run is from |
2 participants
Motivation
While reproducing #1569 I found that
Level's logarithmic operators are exercised byLevelTestsBase.ArithmeticOperatorsfor the ordinary case, but not where the linear resultstops being positive. Two such cases return silently today:
The second is inconsistent with the rest of the quantity.
Level(double quantity, double reference)rejects a non-positive ratio withArgumentOutOfRangeExceptionand the message"The base-10 logarithm of a number ≤ 0 is undefined", while
operator-performs that samelogarithm and yields
NaNwithout signalling anything.Changes
Three tests in
UnitsNet.Tests/CustomCode/LevelTests.cs:LogarithmicSubtraction_OfEqualLevels_ReturnsNegativeInfinityLogarithmicSubtraction_WhenSubtrahendIsLarger_ReturnsNaNLogarithmicAddition_OfTwoLevels_CombinesThemInLinearSpace, using the values from Calculations with db are wrong #1569 andasserting against
10 * log10(10^0.16 + 10^0.07)rather than a bare literal, so the teststates why the answer is what it is.
What this does not do
It changes no behaviour, and none of these tests fail without a source change. They are
characterisation tests: they pin what the operators do today so that any later change to it is
a visible decision rather than a silent one. I have deliberately not made
operator-throw —that is a breaking change and your call, not something to slip in under a test PR. If you would
like it to throw, say so and I will open that separately with these tests updated.
Nothing is asserted about
AmplitudeRatioorPowerRatio, which have the same logarithmicmachinery with a scaling factor of 20 and 10 respectively. I stopped at
Levelbecause that iswhat #1569 is about; happy to extend if useful.
On #1569 itself
I do not think it is a bug, and I have commented there with the measurement.
Levelislogarithmic, so
+combines two levels in linear power space;1.6 dB + 0.7 dBis4.18357203248652 dB, which matches10·log10(10^0.16 + 10^0.07)exactly. The reporter wantedplain arithmetic on the decibel numbers, which is the right operation for a gain rather than a
level, and already works via
Level.FromDecibels(a.Decibels + b.Decibels).Validation
Local run on .NET 10 only; I have not run the full multi-target matrix.