Turn a negated difference round wherever it sits (#882)

BREAKING-CHANGES.md

@@ -71,6 +71,8 @@ read first.
 | **silent** | `ln(e^x)`, `log(2, 2^x)`, `ln(x^2)` | `x`, `x`, `2 * ln(x)` — wrong off the real line | left as written unless the argument is decidable |
 | **silent** | two limits over `(x^2)^x` and `x^x` | answered correctly | unevaluated — a deliberate loss |
 | **silent** | `DirectChildren` of a conditional set | a name off the predicate's hash, and one in 26^4 threw | `%1`, fresh by construction |
+| **silent** | `-(a - b)` inside a power, a function or a matrix | left as written | `b - a`, as at the root |
+| **silent** | `Expand` of a matrix | the matrix, unexpanded | expanded entry by entry |
 | **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` |
@@ -405,6 +407,45 @@ have their own test asserting the unevaluated node, so a future fix flips them b
 `ln(x) + ln(x+1)`, both recorded elsewhere as wanting a decision rather than a guard. Issue
 [#902](https://github.com/asc-community/AngouriMath/issues/902).
 
+### A negated difference is turned round wherever it sits, and `Expand` descends into a matrix
+
+`-(a - b)` became `b - a` for a whole expression and not for the same expression inside another node,
+so what a caller got depended on where the subexpression sat:
+
+| input | was | is |
+|---|---|---|
+| `-(5 - sqrt(-11))` | `sqrt(-11) - 5` | unchanged |
+| `-(5 - sqrt(-11)) + y` | `y - (5 - sqrt(-11))` | `sqrt(-11) - 5 + y` |
+| `2 ^ (-(5 - sqrt(-11)))` | left as written | `2 ^ (sqrt(-11) - 5)` |
+| `sgn(-(5 - sqrt(-11)))` | left as written | `sgn(sqrt(-11) - 5)` |
+| `[[-(5 - sqrt(-11)), 1]]` | left as written | `[[sqrt(-11) - 5, 1]]` |
+| `Expand` of `[[(x+1)^2, 1]]` | `[[(x + 1) ^ 2, 1]]` | `[[1 + 2 * x + x ^ 2, 1]]` |
+
+Every one of these was already the right number, written the long way round. The step came from
+`Expand`, which the simplifier offers for the **root** expression only and which does not descend into
+an exponent, a function's argument or a matrix. Written as a rule instead — a unary minus parses as
+`(-1) * x`, so `(-1) * (a - b)` becomes `b - a`, which is five nodes for three — it runs wherever the
+shape occurs, since a rule walks the tree.
+
+`Expand` of a matrix is the same story from the other end: it read its argument as a sum, and a matrix
+is not one, so a matrix left through the "too complicated, return what came in" exit. It now expands
+entry by entry. `Factorize` and `Differentiate` both already descended, being built out of rewrite
+rules, so this was `Expand` being the odd one out rather than matrices being held back deliberately.
+
+**Where a caller meets it.** `EquationSystem.Solve` returns a `Matrix`, so a solved system's entries
+were left in whatever form the solver built them in:
+
+```
+MathS.Equations("x2 + y", "y - x - 3").Solve("x", "y").Simplify()
+
+was:  [[-(-(5 - sqrt(-11)) / 2 + 3), (5 - sqrt(-11)) / 2], [-(-(5 + sqrt(-11)) / 2 + 3), (5 + sqrt(-11)) / 2]]
+is:   [[-1/2 + -1/2 * sqrt(-11), 5/2 + -1/2 * sqrt(-11)], [1/2 * sqrt(-11) + -1/2, 1/2 * sqrt(-11) + 5/2]]
+```
+
+Issue [#882](https://github.com/asc-community/AngouriMath/issues/882), which
+[#497](https://github.com/asc-community/AngouriMath/issues/497) names as the shape of defect to hunt:
+the same input simplifying or not depending on its parent.
+
 ### A conditional set's bound variable is renamed to a temporary
 
 `DirectChildren` of `{ x : P(x) }` renames the binder, so that the bound `x` is not read as an `x`

Sources/AngouriMath/Functions/Evaluation/Evaluation.Definition.cs

@@ -214,6 +214,16 @@ public Entity Expand(int level = 2)
         /// </summary>
         internal Entity ExpandOverSum(int level)
         {
+            // A matrix is expanded entry by entry. What follows reads the expression as a sum, and
+            // a matrix is not one, so it left through the escape at the bottom and came back as it
+            // arrived: [[(x+1)^2, 1]] was not expanded while (x+1)^2 was. Factorize and
+            // Differentiate both descend into a matrix, being built out of rewrite rules, and a
+            // rule walks the tree -- so this was Expand being the odd one out rather than matrices
+            // being held back on purpose.
+            // https://github.com/asc-community/AngouriMath/issues/882
+            if (this is Matrix matrix)
+                return matrix.With((_, _, entry) => entry.Expand(level));
+
             static Entity Expand_(Entity e, int level) =>
                 level <= 1
                 ? e.Rewrite(RewriteRules.Expansion)

Sources/AngouriMath/Functions/Simplification/Patterns/Patterns.Power.cs

@@ -17,11 +17,28 @@ internal static Entity InvertNegativePowers(Entity expr) =>
             expr is Powf(var @base, Integer { IsNegative: true } pow)
             ? 1 / MathS.Pow(@base, -1 * pow)
             : expr;
-        /// <summary>1 + (-x) => 1 - x</summary>
-        internal static Entity InvertNegativeMultipliers(Entity expr) =>
-            expr is Sumf(var any1, Mulf(Real { IsNegative: true } const1, var any2))
-            ? any1 - (-1 * const1) * any2
-            : expr;
+        /// <summary>1 + (-x) => 1 - x, and -(a - b) => b - a</summary>
+        internal static Entity InvertNegativeMultipliers(Entity expr) => expr switch
+        {
+            Sumf(var any1, Mulf(Real { IsNegative: true } const1, var any2))
+                => any1 - (-1 * const1) * any2,
+
+            // -(a - b) => b - a. A unary minus parses as (-1) * x, so this is the shape a negated
+            // difference arrives in, and turning it round removes the multiplication and the
+            // negative constant together: five nodes become three, and the metric charges four for
+            // a negative real on top of that.
+            //
+            // What matters is where it runs rather than what it does. Expand already produced this
+            // form for a whole expression, and Expand is offered for the root only and does not
+            // descend into an exponent, a function's argument or a matrix -- so `-(5 - sqrt(-11))`
+            // simplified while `2 ^ (-(5 - sqrt(-11)))`, `sgn(-(5 - sqrt(-11)))` and the same entry
+            // inside a matrix did not. A rule runs everywhere.
+            // https://github.com/asc-community/AngouriMath/issues/882
+            Mulf(Integer(-1), Minusf(var subtrahend, var minuend))
+                => minuend - subtrahend,
+
+            _ => expr
+        };
 
         internal static Entity PowerRules(Entity x) => x switch
         {

Sources/Tests/UnitTests/Common/SimplificationRegressionTest.cs

@@ -819,5 +819,62 @@ public void ComposingAbsAndSignumKeepsItsValueAtZero(string expression)
             Assert.Equal(original.Substitute("x", 0).EvalNumerical(),
                 simplified.Substitute("x", 0).EvalNumerical());
         }
+
+        // https://github.com/asc-community/AngouriMath/issues/882
+        // -(a - b) was turned into b - a for a whole expression and not for the same expression
+        // inside another node, because the step came from Expand -- which is offered for the root
+        // only and does not descend into an exponent, a function's argument or a matrix. So what a
+        // caller got depended on where the subexpression sat. As a rule it runs everywhere.
+        //
+        // The printed form is the bug here, which is why it is what these assert: every one of
+        // these was already the right *number*, written the long way round.
+        [Theory]
+        [InlineData("-(5 - sqrt(-11))", "sqrt(-11) - 5")]
+        [InlineData("-(5 - sqrt(-11)) + y", "sqrt(-11) - 5 + y")]
+        [InlineData("2 ^ (-(5 - sqrt(-11)))", "2 ^ (sqrt(-11) - 5)")]
+        [InlineData("sgn(-(5 - sqrt(-11)))", "sgn(sqrt(-11) - 5)")]
+        [InlineData("[[-(5 - sqrt(-11)), 1]]", "[[sqrt(-11) - 5, 1]]")]
+        [InlineData("-(a - b)", "b - a")]
+        public void ANegatedDifferenceIsTurnedRoundWhereverItSits(string expression, string expected) =>
+            Assert.Equal(expected, expression.ToEntity().Simplify().Stringize());
+
+        // And it is the same number afterwards, which the printed forms above do not check.
+        [Theory]
+        [InlineData("-(a - b)")]
+        [InlineData("2 ^ (-(a - b))")]
+        [InlineData("sgn(-(a - b))")]
+        public void TurningANegatedDifferenceRoundKeepsItsValue(string expression)
+        {
+            var original = expression.ToEntity();
+            var simplified = original.Simplify();
+            foreach (var (a, b) in new[] { (2, 7), (-3, 5), (0, 0), (4, -1) })
+                Assert.Equal(
+                    original.Substitute("a", a).Substitute("b", b).EvalNumerical(),
+                    simplified.Substitute("a", a).Substitute("b", b).EvalNumerical());
+        }
+
+        // The same issue's second half: Expand read its argument as a sum, and a matrix is not one,
+        // so a matrix left by the "too complicated, return what came in" exit. Factorize and
+        // Differentiate both descend, being built out of rewrite rules, so Expand was the odd one
+        // out rather than matrices being held back deliberately.
+        [Theory]
+        [InlineData("[[(x+1)^2, 1]]", "[[1 + 2 * x + x ^ 2, 1]]")]
+        [InlineData("[[(x+1)^2, (y+2)^2], [1, x]]", "[[1 + 2 * x + x ^ 2, 4 + 4 * y + y ^ 2], [1, x]]")]
+        public void ExpandDescendsIntoAMatrix(string expression, string expected) =>
+            Assert.Equal(expected, expression.ToEntity().Expand().Stringize());
+
+        // What a user meets it through: EquationSystem.Solve returns a Matrix, so every entry of a
+        // system's answer was left in the form the solver happened to build it in. Asserted by
+        // node count rather than by form, since the point is that the entries got shorter.
+        [Fact]
+        public void ASystemsAnswerSimplifiesEntryByEntry()
+        {
+            var answer = MathS.Equations("x2 + y", "y - x - 3").Solve("x", "y");
+            Assert.NotNull(answer);
+            var simplified = answer!.Simplify();
+            Assert.True(simplified.Complexity < answer.Complexity,
+                $"the system's answer simplified to {simplified.Stringize()}, which is no shorter "
+                + $"than the {answer.Stringize()} it came from");
+        }
     }
 }