diff --git a/BREAKING-CHANGES.md b/BREAKING-CHANGES.md index a7983f90e..f5bb6eb44 100644 --- a/BREAKING-CHANGES.md +++ b/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` diff --git a/Sources/AngouriMath/Functions/Evaluation/Evaluation.Definition.cs b/Sources/AngouriMath/Functions/Evaluation/Evaluation.Definition.cs index 479a7fa55..985aa271e 100644 --- a/Sources/AngouriMath/Functions/Evaluation/Evaluation.Definition.cs +++ b/Sources/AngouriMath/Functions/Evaluation/Evaluation.Definition.cs @@ -214,6 +214,16 @@ public Entity Expand(int level = 2) /// 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) diff --git a/Sources/AngouriMath/Functions/Simplification/Patterns/Patterns.Power.cs b/Sources/AngouriMath/Functions/Simplification/Patterns/Patterns.Power.cs index 2f19b0296..b156debec 100644 --- a/Sources/AngouriMath/Functions/Simplification/Patterns/Patterns.Power.cs +++ b/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; - /// 1 + (-x) => 1 - x - internal static Entity InvertNegativeMultipliers(Entity expr) => - expr is Sumf(var any1, Mulf(Real { IsNegative: true } const1, var any2)) - ? any1 - (-1 * const1) * any2 - : expr; + /// 1 + (-x) => 1 - x, and -(a - b) => b - a + 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 { diff --git a/Sources/Tests/UnitTests/Common/SimplificationRegressionTest.cs b/Sources/Tests/UnitTests/Common/SimplificationRegressionTest.cs index 2107bee1a..06e112faa 100644 --- a/Sources/Tests/UnitTests/Common/SimplificationRegressionTest.cs +++ b/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"); + } } }