diff --git a/.gitignore b/.gitignore index dde550a6e..e220b1c09 100644 --- a/.gitignore +++ b/.gitignore @@ -14,16 +14,9 @@ obj/ /obj /packages /fieldtest/testprj -/AngouriMath/Core/Sys/Items/Numeric/packages -/Tests/UnitTests/Algebra/SolveTest/TestGenerator/.idea -/Tests/UnitTests/Algebra/SolveTest/TestGenerator/out/production/AM_utils -/Tests/UnitTests/TestGenerator/.idea -/Tests/UnitTests/TestUtils/.idea -/Tests/UnitTests/TestUtils/out/production/TestGenerator /AngouriMath.sln.DotSettings.user -/Tests/UnitTests/.idea -/Tests/DotnetBenchmark/benchmark_results.csv -/AngouriMath/Core/Antlr/antlr4 +/Sources/Tests/DotnetBenchmark/benchmark_results.csv +/Sources/AngouriMath/Core/Antlr/antlr4 /Samples/.ipynb_checkpoints /Sources/Samples/.ipynb_checkpoints /Sources/Samples/CPlusPlusPlayground/load.obj diff --git a/BREAKING-CHANGES.md b/BREAKING-CHANGES.md index 23b93cb4b..306a9046f 100644 --- a/BREAKING-CHANGES.md +++ b/BREAKING-CHANGES.md @@ -67,6 +67,7 @@ read first. | loud | a polynomial system with more equations than unknowns | `WrongNumberOfArgumentsException` | solved | | loud | a known gap, e.g. a cubic inequality | `AngouriBugException`, asking to be reported | `NotSufficientlySupportedException` | | **silent** | `arcsin(sin(x))` and three siblings | `x`, wrong wherever `x` leaves the principal interval | left as written unless `x` is a real in that interval | +| **silent** | `abs(sgn(x))` and `sgn(abs(x))` | `1`, wrong at `x = 0` where both are `0` | left as written unless the argument's value can be read | | **silent** | `arctan(x) + arccotan(x)` | `pi/2`, wrong for every negative `x` | `pi/2` or `-pi/2` where the sign is known, else left as written | | **silent** | `log(1, 1)` | `0` | `NaN`, since it is `0/0` | | **silent** | `log(b, 1)` | `0` for any base | `0 provided not b = 1` | @@ -349,6 +350,39 @@ Found by `boundcheck`, a harness that composes every unary function node with ev compares against the original at points where an assumption fails rather than at sampled points. Issue [#887](https://github.com/asc-community/AngouriMath/issues/887). +### `abs(sgn(x))` and `sgn(abs(x))` are not `1` at zero + +`|sgn(z)|` and `sgn(|z|)` are `1` for every `z` except `0`, where both are `0`, because `sgn(0)` is +`0`. Both rewrites answered `1` for any argument. + +| | was | is | +|---|---|---| +| `abs(sgn(x))`, `sgn(abs(x))` | `1` | left as written | +| the same at `x = 0` | `1` | `0` — which is the value | +| `abs(sgn(0))`, `sgn(abs(0))` | `0`, unchanged | | +| `abs(sgn(2))`, `sgn(abs(-3))` | `1`, unchanged | | +| `signum(abs(x/x))`, `abs(signum(x/x))` | `1 provided not x = 0` | left as written | + +The rules consulted the argument's `DomainCondition`, which answers a different question from the one +they needed: a bare `x` is defined everywhere and can *be* zero, while `x / x` is defined only away +from zero and is nonzero throughout. Reading the domain conflated the two. + +**A condition would not have been the fix**, for the same reason as in the two entries above: the +expression is defined at zero and equal to `0` there, so `1 provided not z = 0` would replace a wrong +value with a wrong domain. There is no closed form for "1 away from zero, 0 at it" other than the +expressions themselves, so the rules decide where the argument's value can be read and decline +otherwise. + +**The last row is a coverage loss, not a correction.** `1 provided not x = 0` was right for `x / x`. +Separating it from the `x` case needs "nonzero throughout its domain", and neither test available in +`InnerSimplify` can express it — `Evaled` leaves `x / x` as `x / x`, and `DomainCondition` is what +conflated them; `Simplify` would answer it and cannot be called from there (#403). + +Found by `boundcheck` after adding `x = 0` and `x = 1` to its sample points: 366 shapes had been +passing at 23 points chosen for branch cuts and principal intervals, none of which was the place +where a rule's own arithmetic degenerates. Issue +[#892](https://github.com/asc-community/AngouriMath/issues/892). + ### A known gap no longer presents as a bug `FutureReleaseException` is removed, and the twelve places that threw through it now throw diff --git a/Sources/AngouriMath/Docs/WhatsNew/version_performance_control.md b/Sources/AngouriMath/Docs/WhatsNew/version_performance_control.md index 2597612ce..fac6275e7 100644 --- a/Sources/AngouriMath/Docs/WhatsNew/version_performance_control.md +++ b/Sources/AngouriMath/Docs/WhatsNew/version_performance_control.md @@ -101,3 +101,59 @@ timings do. argument for it. Evaluating `1 + 2 + log(2, 3) + sqrt(4) - 4 ^ 7 + e * pi` cannot allocate zero bytes; a dictionary lookup can. `RunEasy`, `RunMedium` and `RunHard` allocate nothing for the honest reason -- a compiled delegate over `Complex` has nothing to put on the heap. + +--- + +## The 1671st and the 1697th, measured together on one machine + +**Why a pair rather than a column.** 2.0.0 shipped without any performance measurement at all, so +there is no column for it: it is commit 1691 and the table above stops at the 1671st. Filling that gap +with a single new column would have been worse than leaving it. Measured on a developer machine, every +one of the sixteen rows came out at 0.5–0.6× of the 1671st — a uniform factor across the board, which +is a faster machine and not faster code. A reader comparing the two columns would have seen an +across-the-board 1.75× improvement that does not exist, which is exactly the mistake the note at the +top of this file warns about. + +So the 1671st was re-measured on the same machine, minutes apart, with nothing else running. Only the +right-hand pair below may be read as a ratio; neither may be compared with the columns above. + +The 1697th is `6476513b`, which is 2.0.0 plus the five corrections merged after it and the `abs`/`sgn` +fix — so this answers "did the release and the fixes after it cost anything", which is the question +2.0.1 needs answered. + +| Method | [1671st](https://github.com/asc-community/AngouriMath/commit/253b5a8d598a9a1d721b777340bd53ae3be12f99) | [1697th](https://github.com/asc-community/AngouriMath/commit/6476513b) | change | +|---|--:|--:|--:| +| ParseEasy | 5,887 ns | 6,052 ns | +2.8% | +| ParseHard | 1,364,570 ns | 1,430,477 ns | +4.8% | +| SimplifyEasy | 79,832 ns | 86,744 ns | **+8.7%** | +| SimplifyHard | 1,646,740,009 ns | 1,697,337,871 ns | +3.1% | +| Derivate | 13,745 ns | 14,367 ns | +4.5% | +| SolveEasy | 11,302,282 ns | 11,883,543 ns | +5.1% | +| SolveEasyMedium | 24,545 ns | 24,438 ns | −0.4% | +| SolveMedium | 420,926 ns | 433,918 ns | +3.1% | +| SolveMediumHard | 74,401,542 ns | 76,053,093 ns | +2.2% | +| SolveHard | 751,607,244 ns | 766,020,069 ns | +1.9% | +| EvalEasy | 1.34 ns | 0.85 ns | see below | +| EvalTrig | 720,189 ns | 711,009 ns | −1.3% | +| EvalTrigPrecise | 22,870,710 ns | 23,086,271 ns | +0.9% | +| CompileEasy | 191,188 ns | 191,289 ns | +0.1% | +| CompileHard | 320,897 ns | 323,255 ns | +0.7% | +| RunEasy | 20.12 ns | 20.24 ns | +0.6% | +| RunMedium | 163.8 ns | 169.9 ns | +3.7% | +| RunHard | 290.6 ns | 296.9 ns | +2.2% | + +Allocation over the same pair: `SimplifyHard` 3,710,538,664 → 3,737,029,480 B (+0.7%), `SolveHard` +1,341,724,616 → 1,356,514,512 B (+1.1%), `SolveMediumHard` 152,244,664 → 155,187,208 B (+1.9%); +everything else within a few tenths of a per cent. + +**Read: no regression, with one row worth watching.** Most of the table moves 1–5%, which by this +file's own standard says nothing. `SimplifyEasy` at **+8.7%** is above that, and there is a plausible +cause rather than a mystery: the corrections merged in that range added guards to the simplification +patterns — `WithinHalfPi`, `WithinArccotanRange`, and the `abs`/`sgn` zero test — and each calls +`Evaled` inside a pattern's `when` clause on a path that previously matched structurally. `Evaled` is +cached per node, so the cost is a first-call computation and a lookup thereafter, but it is not free +and it is on the hot path. Worth a measurement of its own before more guards are written that way. + +`EvalEasy` is not a real movement in either direction. It measures a cached lookup rather than +arithmetic — see the note above the main table — and at one nanosecond the figure is dominated by +whatever the harness itself costs. diff --git a/Sources/AngouriMath/Functions/Evaluation/Evaluation.Continuous/Evaluation.Continuous.Arithmetics.Classes.cs b/Sources/AngouriMath/Functions/Evaluation/Evaluation.Continuous/Evaluation.Continuous.Arithmetics.Classes.cs index c5972d1f9..e34d92872 100644 --- a/Sources/AngouriMath/Functions/Evaluation/Evaluation.Continuous/Evaluation.Continuous.Arithmetics.Classes.cs +++ b/Sources/AngouriMath/Functions/Evaluation/Evaluation.Continuous/Evaluation.Continuous.Arithmetics.Classes.cs @@ -12,6 +12,27 @@ namespace AngouriMath { partial record Entity { + /// + /// The value of where it has one, with the condition it holds + /// under, or where there is no value to read. + /// + /// + /// The condition is what separates an argument that is nonzero *throughout its domain* + /// from one that can itself vanish. x / x evaluates to 1 provided not x = 0, + /// so it is nonzero wherever it is defined at all and a rule may rely on that, carrying + /// the condition forward; a bare x has no value and no such promise. Reading only + /// the argument's conflates the two, which is how + /// sgn(abs(x)) came to be 1 + /// (https://github.com/asc-community/AngouriMath/issues/892). + /// + private static (Number.Complex Value, Entity Condition)? ValueWithCondition(Entity expr) + => expr.Evaled switch + { + Number.Complex value => (value, Boolean.True), + Providedf(Number.Complex value, var condition) => (value, condition), + _ => null, + }; + public partial record Number { private protected override Entity IntrinsicCondition => Boolean.True; @@ -278,7 +299,19 @@ protected override Entity InnerSimplify(bool isExact) { Real n => n.EDecimal.Sign, Complex n when !isExact => Number.Signum(n), - Absf({ DomainCondition: var condition }) => Integer.One.Provided(condition), + // sgn(|z|) is 1 wherever z is nonzero and 0 where it is not: |z| is never + // negative, so the only question is whether it vanishes. This answered 1 + // for every argument -- so sgn(abs(x)) simplified to 1, where at x = 0 the + // value is 0, because sgn(0) is 0 as the comment above says. + // + // A condition is not the fix. The expression is defined at zero and equal + // to 0 there, so `1 provided not z = 0` would replace a wrong value with a + // wrong domain. Where the argument's vanishing cannot be decided the node + // is left as written instead. + // https://github.com/asc-community/AngouriMath/issues/892 + Absf(var magnitudeOf) when ValueWithCondition(magnitudeOf) is { } known + => (known.Value.IsZero ? Integer.Zero : Integer.One) + .Provided(known.Condition), Signumf signum => signum, _ => null }, @@ -447,7 +480,13 @@ protected override Entity InnerSimplify(bool isExact) Matrix m when m.IsVector => VectorNorm(m, isExact), Complex n when !isExact => Number.Abs(n), Absf abs => abs, - Signumf({ DomainCondition: var condition }) => Integer.One.Provided(condition), + // |sgn(z)| has unit modulus wherever z is nonzero and is 0 at zero, the + // same question as sgn(|z|) above and with the same answer. See there for + // why this decides rather than attaching a condition. + // https://github.com/asc-community/AngouriMath/issues/892 + Signumf(var signOf) when ValueWithCondition(signOf) is { } known + => (known.Value.IsZero ? Integer.Zero : Integer.One) + .Provided(known.Condition), _ => null }, (@this, a) => ((Absf)@this).New(a), isExact); diff --git a/Sources/Tests/UnitTests/Common/InnerSimplifyTest.cs b/Sources/Tests/UnitTests/Common/InnerSimplifyTest.cs index 9e5c3c66c..34a503063 100644 --- a/Sources/Tests/UnitTests/Common/InnerSimplifyTest.cs +++ b/Sources/Tests/UnitTests/Common/InnerSimplifyTest.cs @@ -22,6 +22,12 @@ public class InnerSimplifyTest [InlineData("ln(abs(x)) - 1")] [InlineData("2 * ln(abs(x))")] [InlineData("ln(abs(x))^2")] + // Both are 0 at x = 0, since sgn(0) is 0, so neither collapses to 1 any more. + // https://github.com/asc-community/AngouriMath/issues/892 + [InlineData("signum(abs(x))")] + [InlineData("abs(signum(x))")] + [InlineData("signum(abs(x/x))")] + [InlineData("abs(signum(x/x))")] public void ShouldNotChangeTest(string expr) { var expected = expr.ToEntity(); @@ -83,10 +89,16 @@ public void ShouldNotChangeTestTodo(string expr) [InlineData("abs(abs(x))", "abs(x)")] [InlineData("signum(signum(x))", "signum(x)")] - [InlineData("signum(abs(x))", "1")] - [InlineData("abs(signum(x))", "1")] - [InlineData("signum(abs(x/x))", "1 provided not x = 0")] - [InlineData("abs(signum(x/x))", "1 provided not x = 0")] + // signum(abs(x)) and abs(signum(x)) asserted 1 and are 0 at x = 0 + // (https://github.com/asc-community/AngouriMath/issues/892), so they are now left as + // written and live in ShouldNotChange below. + // + // The x/x pair went with them, and that is a coverage loss rather than a correction: + // x/x is nonzero everywhere it is defined, so 1 provided not x = 0 was right for it. + // Telling the two apart needs "nonzero throughout its domain", and the only tests + // available here are the argument's Evaled -- which leaves x/x as x/x -- and its + // DomainCondition, which is what conflated them in the first place. Simplify would + // answer it and cannot be called from inside InnerSimplify (#403). [InlineData("log(10, x) * log(10, x)", "log(10, x)^2")] public void ShouldChangeTo(string from, string to) diff --git a/Sources/Tests/UnitTests/Common/SimplificationRegressionTest.cs b/Sources/Tests/UnitTests/Common/SimplificationRegressionTest.cs index 10be70359..675df53fa 100644 --- a/Sources/Tests/UnitTests/Common/SimplificationRegressionTest.cs +++ b/Sources/Tests/UnitTests/Common/SimplificationRegressionTest.cs @@ -699,5 +699,44 @@ public void ArctanPlusArccotanOfASymbolIsLeftAlone() Assert.NotEqual(MathS.pi / 2, simplified); Assert.NotEqual(-MathS.pi / 2, simplified); } + + // https://github.com/asc-community/AngouriMath/issues/892 + // |sgn(z)| and sgn(|z|) are 1 for every z except 0, where both are 0 -- sgn(0) is 0, + // which the comment above Signumf.InnerSimplify already said. Both rewrites answered 1 + // for any argument at all, so a symbolic one gave a value that is wrong at one point. + // + // A condition would be the wrong fix: at z = 0 the expression is defined and equal to + // 0, so `1 provided not z = 0` would trade a wrong value for a wrong domain. + [Theory] + [InlineData("abs(sgn(x))")] + [InlineData("sgn(abs(x))")] + [InlineData("abs(sgn(x + y))")] + public void ComposingAbsAndSignumOverASymbolIsLeftAlone(string expression) => + Assert.NotEqual(Entity.Number.Integer.Create(1), expression.ToEntity().Simplify()); + + // Where the argument's vanishing can be decided, the answer is still given, and it is + // the right one at zero. + [Theory] + [InlineData("abs(sgn(0))", "0")] + [InlineData("sgn(abs(0))", "0")] + [InlineData("abs(sgn(2))", "1")] + [InlineData("sgn(abs(-3))", "1")] + [InlineData("abs(sgn(-1/2))", "1")] + [InlineData("sgn(abs(i))", "1")] + public void ComposingAbsAndSignumOverANumberIsDecided(string expression, string expected) => + Assert.Equal(expected.ToEntity(), expression.ToEntity().Simplify()); + + // The value is what the bug was, so it is the value that is asserted: at x = 0 both + // compositions are 0, and a rewrite to 1 is off by one there. + [Theory] + [InlineData("abs(sgn(x))")] + [InlineData("sgn(abs(x))")] + public void ComposingAbsAndSignumKeepsItsValueAtZero(string expression) + { + var original = expression.ToEntity(); + var simplified = original.Simplify(); + Assert.Equal(original.Substitute("x", 0).EvalNumerical(), + simplified.Substitute("x", 0).EvalNumerical()); + } } } diff --git a/Sources/Tests/UnitTests/PatternsTest/SimplifyTest.cs b/Sources/Tests/UnitTests/PatternsTest/SimplifyTest.cs index 9546f23f9..182f5469d 100644 --- a/Sources/Tests/UnitTests/PatternsTest/SimplifyTest.cs +++ b/Sources/Tests/UnitTests/PatternsTest/SimplifyTest.cs @@ -152,8 +152,15 @@ [Fact] public void BigSimple1() => AssertSimplifyToString( [InlineData("abs(x)", "abs(x)")] [InlineData("abs(abs(x))", "abs(x)")] [InlineData("abs(abs(abs(x)))", "abs(x)")] - [InlineData("abs(sgn(x))", "1")] - [InlineData("sgn(abs(x))", "1")] + // Both of these asserted 1, and both are 0 at x = 0, because sgn(0) is 0. They are left + // as written now: a condition would say the expression is undefined at zero when it is + // merely something else, and there is no closed form for "1 away from zero, 0 at it" + // other than the expressions themselves. + // https://github.com/asc-community/AngouriMath/issues/892 + [InlineData("abs(sgn(x))", "abs(sgn(x))")] + [InlineData("sgn(abs(x))", "sgn(abs(x))")] + [InlineData("abs(sgn(0))", "0")] + [InlineData("sgn(abs(-3))", "1")] public void AbsTest(string input, string output) => AssertSimplifyToString(input, output); [Theory]