Read abs and signum at zero, where both compositions are 0 (#892)

.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

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

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.

Sources/AngouriMath/Functions/Evaluation/Evaluation.Continuous/Evaluation.Continuous.Arithmetics.Classes.cs

@@ -12,6 +12,27 @@ namespace AngouriMath
 {
     partial record Entity
     {
+        /// <summary>
+        /// The value of <paramref name="expr"/> where it has one, with the condition it holds
+        /// under, or <see langword="null"/> where there is no value to read.
+        /// </summary>
+        /// <remarks>
+        /// The condition is what separates an argument that is nonzero *throughout its domain*
+        /// from one that can itself vanish. <c>x / x</c> evaluates to <c>1 provided not x = 0</c>,
+        /// so it is nonzero wherever it is defined at all and a rule may rely on that, carrying
+        /// the condition forward; a bare <c>x</c> has no value and no such promise. Reading only
+        /// the argument's <see cref="DomainCondition"/> conflates the two, which is how
+        /// <c>sgn(abs(x))</c> came to be <c>1</c>
+        /// (https://github.com/asc-community/AngouriMath/issues/892).
+        /// </remarks>
+        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);

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)

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());
+        }
     }
 }

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]