diff --git a/BREAKING-CHANGES.md b/BREAKING-CHANGES.md index 3e48af284..87aef94bf 100644 --- a/BREAKING-CHANGES.md +++ b/BREAKING-CHANGES.md @@ -65,6 +65,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** | `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 | --- @@ -240,6 +241,47 @@ when in fact it merely has another value — trading a wrong value for a wrong d asserts that the expression is left alone. Issue [#884](https://github.com/asc-community/AngouriMath/issues/884). +### `arctan(x) + arccotan(x)` is not always `pi/2`, and `arccotan(cotan(x))` was guarded wrongly + +Both follow from one fact about this library's `arccotan`: it is `arctan(1/x)` extended by +`arccotan(0) = pi/2`, so its **range is `(-pi/2, pi/2]`** and not the `(0, pi)` that many textbooks +use. Measured: `arccotan(1)` is `pi/4`, `arccotan(-1)` is `-pi/4`, `arccotan(0)` is `pi/2`. + +| | was | is | +|---|---|---| +| `arctan(-3) + arccotan(-3)` | `pi/2` | `-1/2 * pi` — which is the value | +| `arctan(3) + arccotan(3)` | `pi/2` | `pi/2`, unchanged | +| `arctan(x) + arccotan(x)` for symbolic `x` | `pi/2` | left as written | +| `arccotan(cotan(2))` | `2` | `-1.1416...` — which is the value, `2 - pi` | +| `arccotan(cotan(-1/2))` | left as written | `-1/2` | + +The sum is `pi/2` for a non-negative real argument and `-pi/2` for a negative one. `pi/2 * sgn(x)` +is the closed form and is wrong at exactly one point — at `x = 0` the sum is `pi/2` while `sgn(0)` +is `0` — so the sign is decided where it can be read and the sum is left alone otherwise. A +`Piecewise` would be total, but `Compile` throws `UncompilableNodeException` on one, so answering +that way would break expressions that compile today. + +**The second row is a correction to the release before it.** 2.0.0 guarded +`arccotan(cotan(x))` with `[0, pi]`, on the assumption that `arccotan`'s range was `(0, pi)`. That +admitted `(pi/2, pi)`, where the rewrite is false, and refused `(-pi/2, 0)`, where it is true. The +interval is now `(-pi/2, pi/2]` without zero — zero excluded because `cotan` has no value there, so +the composition has none either and rewriting to `x` would invent one. The other three intervals +from that change check out: `arcsin` is `[-pi/2, pi/2]`, `arccos` is `[0, pi]`, `arctan` is +`(-pi/2, pi/2)`. + +`arcsin(x) + arccos(x) -> pi/2` is **unchanged and needs no assumption**, since `arccos(x)` is +`pi/2 - arcsin(x)` by definition over the whole plane. + +Three tests moved. `SimplifyTest.Patt8` and `ArctanIdentitiesTest.NeighbouringIdentitiesAreUnaffected` +asserted `pi/2` for a symbolic argument and were pinning the wrong answer; both now use numbers and +cover each sign. `SortSimplifyTest`'s `arctan(x2) + arccot(x*x)` case sorted and collected its whole +sum only because the collapse shortened it, so with the collapse gone the sum stays as written — +the sibling `arcsin`/`arccos` case still collapses and still sorts. + +Found by `boundcheck`, a harness that composes every unary function node with every other and +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). + ### 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/Functions/Simplification/Patterns/Patterns.Trigonometry.cs b/Sources/AngouriMath/Functions/Simplification/Patterns/Patterns.Trigonometry.cs index e0dbe5126..49c0218d6 100644 --- a/Sources/AngouriMath/Functions/Simplification/Patterns/Patterns.Trigonometry.cs +++ b/Sources/AngouriMath/Functions/Simplification/Patterns/Patterns.Trigonometry.cs @@ -43,9 +43,46 @@ private static bool WithinHalfPi(Entity argument, bool closed) return closed ? comparison <= 0 : comparison < 0; } + /// Whether the argument lies in the range arccotan answers in, which is + /// (-pi/2, pi/2] without zero. + /// + /// This library's arccotan is arctan(1/x) extended by + /// arccotan(0) = pi/2, so its range is that half-open interval and **not** the + /// (0, pi) that many texts use — arccotan(-1) is -pi/4. Zero is + /// excluded because cotan has no value there, so the composition has none either + /// and rewriting it to x would invent one. + /// https://github.com/asc-community/AngouriMath/issues/887 + /// + private static bool WithinArccotanRange(Entity argument) + { + if (!TryReadReal(argument, out var value) || value.IsZero) return false; + var twice = value.Add(value); + var pi = MathS.DecimalConst.pi; + return twice.CompareTo(pi) <= 0 && twice.CompareTo(pi.Negate()) > 0; + } + + /// + /// arctan(x) + arccotan(x), which is pi/2 where x is a non-negative + /// real and -pi/2 where it is negative, or where the sign + /// cannot be read. + /// + /// + /// It follows from the range above: for positive x both terms are positive and sum + /// to pi/2, and for negative x both are negative. pi/2 * sgn(x) is + /// the closed form and is wrong at exactly one point — x = 0, where the sum is + /// pi/2 while sgn is 0 — so the sign is decided here rather than + /// written into the answer. A symbolic argument has no decidable sign and is left alone. + /// + private static Entity? ArctanPlusArccotan(Entity argument) + { + var evaled = argument.Evaled; + if (IsRealNegative(evaled)) return -MathS.pi / 2; + if (IsRealPositive(evaled) || IsZero(evaled)) return MathS.pi / 2; + return null; + } + /// Whether the argument lies in [0, pi], or in the open interval when - /// is false — the intervals arccos and - /// arccotan answer in. + /// is false — the interval arccos answers in. private static bool WithinZeroAndPi(Entity argument, bool closed) { if (!TryReadReal(argument, out var value)) return false; @@ -61,11 +98,19 @@ private static bool WithinZeroAndPi(Entity argument, bool closed) Mulf(Sinf(var any1), Cosf(var any1a)) when any1 == any1a => Rational.Create(1, 2) * new Sinf(2 * any1), Mulf(Cosf(var any1), Sinf(var any1a)) when any1 == any1a => Rational.Create(1, 2) * new Sinf(2 * any1), - // arc1({}) + arc2({}) = pi/2 + // arcsin(x) + arccos(x) = pi/2 wherever both are defined, and needs no assumption: + // arccos(x) is pi/2 - arcsin(x) by definition, over the whole plane. Sumf(Arcsinf(var any1), Arccosf(var any1a)) when any1 == any1a => MathS.pi / 2, Sumf(Arccosf(var any1), Arcsinf(var any1a)) when any1 == any1a => MathS.pi / 2, - Sumf(Arctanf(var any1), Arccotanf(var any1a)) when any1 == any1a => MathS.pi / 2, - Sumf(Arccotanf(var any1), Arctanf(var any1a)) when any1 == any1a => MathS.pi / 2, + + // arctan(x) + arccotan(x) does *not* behave that way here, because arccotan is + // arctan(1/x) with range (-pi/2, pi/2]: the sum is pi/2 for non-negative x and + // -pi/2 for negative x. It was pi/2 unconditionally, which is a wrong answer at + // every negative real -- https://github.com/asc-community/AngouriMath/issues/887 + Sumf(Arctanf(var any1), Arccotanf(var any1a)) when any1 == any1a + && ArctanPlusArccotan(any1) is { } sum => sum, + Sumf(Arccotanf(var any1), Arctanf(var any1a)) when any1 == any1a + && ArctanPlusArccotan(any1) is { } sum => sum, // arctan(a) + arctan(b) = arctan((a + b)/(1 - ab)), which is the tangent // addition formula read backwards. It holds as written only while ab < 1: past @@ -118,7 +163,7 @@ private static bool WithinZeroAndPi(Entity argument, bool closed) Arcsinf(Sinf(var any1)) when WithinHalfPi(any1, closed: true) => any1, Arccosf(Cosf(var any1)) when WithinZeroAndPi(any1, closed: true) => any1, Arctanf(Tanf(var any1)) when WithinHalfPi(any1, closed: false) => any1, - Arccotanf(Cotanf(var any1)) when WithinZeroAndPi(any1, closed: false) => any1, + Arccotanf(Cotanf(var any1)) when WithinArccotanRange(any1) => any1, // func(arcfunc(x)) = x, and this direction needs no assumption: it composes the // *right* inverse, so sin(arcsin(z)) is z wherever arcsin(z) is defined at all. diff --git a/Sources/Tests/UnitTests/Common/ArctanIdentitiesTest.cs b/Sources/Tests/UnitTests/Common/ArctanIdentitiesTest.cs index 9c78fdf3b..99d26ea7d 100644 --- a/Sources/Tests/UnitTests/Common/ArctanIdentitiesTest.cs +++ b/Sources/Tests/UnitTests/Common/ArctanIdentitiesTest.cs @@ -70,9 +70,16 @@ public void ASimplifiedSumIsTheSameNumber() } // The identities next to this one have to keep working. + // + // arctan(x) + arccotan(x) is *not* among them and was listed here in error: this + // library's arccotan is arctan(1/x) with range (-pi/2, pi/2], so the sum is pi/2 only + // for non-negative x and is -pi/2 for negative x. It is now answered where the sign is + // decidable and left alone for a symbol, so the case below uses a number. + // https://github.com/asc-community/AngouriMath/issues/887 [Theory] [InlineData("arcsin(x) + arccos(x)", "pi / 2")] - [InlineData("arctan(x) + arccotan(x)", "pi / 2")] + [InlineData("arctan(3) + arccotan(3)", "pi / 2")] + [InlineData("arctan(-3) + arccotan(-3)", "-pi / 2")] [InlineData("tan(arctan(x))", "x")] [InlineData("arctan(tan(1/2))", "1/2")] public void NeighbouringIdentitiesAreUnaffected(string expression, string expected) => diff --git a/Sources/Tests/UnitTests/Common/SimplificationRegressionTest.cs b/Sources/Tests/UnitTests/Common/SimplificationRegressionTest.cs index 01192428a..0743c8855 100644 --- a/Sources/Tests/UnitTests/Common/SimplificationRegressionTest.cs +++ b/Sources/Tests/UnitTests/Common/SimplificationRegressionTest.cs @@ -585,5 +585,75 @@ public void ComposingAFunctionOverItsOwnInverseIsStillTheIdentity(string input) static double Magnitude(Entity difference) => ((System.Numerics.Complex)difference.EvalNumerical()).Magnitude; + // This library's arccotan is arctan(1/x) extended with arccotan(0) = pi/2, so its + // range is (-pi/2, pi/2] and not the (0, pi) some texts use. #884 guarded + // arccotan(cotan(x)) with [0, pi] on the assumption it was the latter, which left the + // wrong answer in place above pi/2 -- arccotan(cotan(2)) is -1.1416 and simplified to + // 2 -- and refused the rewrite below zero, where it is correct. + // The argument has to be a number *before* Simplify runs, because that is what the + // guard reads: a symbolic argument leaves the node alone and would pass whatever the + // interval said. Every one of these is a composition whose value the rewrite must not + // change, at points inside and outside each principal range. + [Theory] + [InlineData("arccotan(cotan(2))")] + [InlineData("arccotan(cotan(3))")] + [InlineData("arccotan(cotan(-2))")] + [InlineData("arccotan(cotan(-1/2))")] + [InlineData("arccotan(cotan(1/2))")] + [InlineData("arcsin(sin(3))")] + [InlineData("arcsin(sin(-3))")] + [InlineData("arcsin(sin(1/2))")] + [InlineData("arccos(cos(4))")] + [InlineData("arccos(cos(-1))")] + [InlineData("arccos(cos(2))")] + [InlineData("arctan(tan(2))")] + [InlineData("arctan(tan(-2))")] + [InlineData("arctan(tan(1/2))")] + public void CancellingAnInverseOverANumberKeepsTheValue(string expression) + { + var original = expression.ToEntity(); + var simplified = original.Simplify(); + Assert.True(Magnitude(original.EvalNumerical() - simplified.EvalNumerical()) < 1e-20, + $"{expression} simplified to {simplified.Stringize()}, which is " + + $"{simplified.EvalNumerical().Stringize()} rather than " + + $"{original.EvalNumerical().Stringize()}"); + } + + // Inside the range it must still cancel, and exactly. -1/2 is in it and 2 is not, + // which is the half the old interval had backwards. + [Theory] + [InlineData("arccotan(cotan(1/2))", "1/2")] + [InlineData("arccotan(cotan(-1/2))", "-1/2")] + [InlineData("arccotan(cotan(-1))", "-1")] + public void SimplifyingArccotanOfCotanStaysExactInsideItsRange(string expression, string expected) + { + Assert.Equal(expected.ToEntity().Simplify(), expression.ToEntity().Simplify()); + } + + // arctan(x) + arccotan(x) is pi/2 for x >= 0 and -pi/2 for x < 0, by the same range. + // It was pi/2 unconditionally. + [Theory] + [InlineData("3", "pi / 2")] + [InlineData("1/2", "pi / 2")] + [InlineData("0", "pi / 2")] + [InlineData("-3", "-pi / 2")] + [InlineData("-1/2", "-pi / 2")] + public void ArctanPlusArccotanFollowsTheSignOfItsArgument(string at, string expected) + { + var sum = $"arctan({at}) + arccotan({at})".ToEntity().Simplify(); + Assert.True(Magnitude(sum - expected.ToEntity()) < 1e-20, + $"arctan({at}) + arccotan({at}) simplified to {sum.Stringize()}, not {expected}"); + } + + // A symbolic argument has no decidable sign, so the sum is left as written rather than + // answered for one sign of it. + [Fact] + public void ArctanPlusArccotanOfASymbolIsLeftAlone() + { + var simplified = "arctan(x) + arccotan(x)".ToEntity().Simplify(); + Assert.NotEqual(MathS.pi / 2, simplified); + Assert.NotEqual(-MathS.pi / 2, simplified); + } + } } diff --git a/Sources/Tests/UnitTests/PatternsTest/SimplifyTest.cs b/Sources/Tests/UnitTests/PatternsTest/SimplifyTest.cs index 6b267dce9..9546f23f9 100644 --- a/Sources/Tests/UnitTests/PatternsTest/SimplifyTest.cs +++ b/Sources/Tests/UnitTests/PatternsTest/SimplifyTest.cs @@ -45,7 +45,16 @@ [Fact] public void Patt2() => AssertSimplify( [Fact] public void Patt5() => AssertSimplify((x + 3) * (3 / (x + 3)), "3 provided not 3 + x = 0"); [Fact] public void Patt6() => AssertSimplify((x + 1) * (x + 2) * (x + 3) / ((x + 2) * (x + 3)), "1 + x provided not (2 + x) * (3 + x) = 0"); [Fact] public void Patt7() => AssertSimplify(MathS.Arcsin(x * 3) + MathS.Arccos(x * 3), MathS.pi / 2); - [Fact] public void Patt8() => AssertSimplify(MathS.Arccotan(x * 3) + MathS.Arctan(x * 3), MathS.pi / 2); + // arccotan here is arctan(1/x) with range (-pi/2, pi/2], so this sum is pi/2 only where + // the argument is a non-negative real and -pi/2 where it is negative. It asserted pi/2 + // for a symbolic argument, which is a wrong answer at every negative x, and the rewrite + // now decides the sign or leaves the sum alone. + // https://github.com/asc-community/AngouriMath/issues/887 + [Fact] public void Patt8() => AssertSimplify(MathS.Arccotan(x * 3) + MathS.Arctan(x * 3), + MathS.Arccotan(3 * x) + MathS.Arctan(3 * x)); + [Fact] public void Patt8Positive() => AssertSimplify(MathS.Arccotan(3) + MathS.Arctan(3), MathS.pi / 2); + [Fact] public void Patt8Negative() => + AssertSimplifyToString(MathS.Arccotan(-3) + MathS.Arctan(-3), "-1/2 * pi"); [Fact] public void Patt9() => AssertSimplify(MathS.Arccotan(x * 3) + MathS.Arctan(x * 6), MathS.Arccotan(3 * x) + MathS.Arctan(6 * x)); [Fact] public void Patt10() => AssertSimplify(MathS.Arcsin(x * 3) + MathS.Arccos(x * 1), MathS.Arcsin(3 * x) + MathS.Arccos(x)); [Fact] public void Patt11() => AssertSimplify(3 + x + 4 + x, 7 + 2 * x); diff --git a/Sources/Tests/UnitTests/PatternsTest/SortSimplifyTest.cs b/Sources/Tests/UnitTests/PatternsTest/SortSimplifyTest.cs index 00a160741..c801be635 100644 --- a/Sources/Tests/UnitTests/PatternsTest/SortSimplifyTest.cs +++ b/Sources/Tests/UnitTests/PatternsTest/SortSimplifyTest.cs @@ -16,7 +16,14 @@ public sealed class SortSimplifyTest [Theory] [InlineData("a + x + e + d + sin(x) + c + 1 + 2 + 2a", "3 + sin(x) + 3 * a + c + d + e + x")] [InlineData("x + a + b + c + arcsin(x2) + d + e + 1/2 - 23 * sqrt(3) + arccos(x * x)", "1/2 + (-23) * sqrt(3) + pi / 2 + a + b + c + d + e + x")] - [InlineData("x + a + b + c + arctan(x2) + d + e + 1/2 - 23 * sqrt(3) + arccot(x * x)", "1/2 + (-23) * sqrt(3) + pi / 2 + a + b + c + d + e + x")] + // The sorting in this case was a side effect of the collapse, not independent of it: + // arctan(x^2) + arccotan(x^2) used to become pi/2, which shortened the sum and made the + // sorted-and-collected candidate the winner. That collapse is only valid where the + // argument's sign is known, and x^2 is not a known non-negative for a symbolic x -- at + // x = i it is -1 -- so the sum now stays and nothing shortens. The arcsin/arccos case + // above still collapses, because that identity needs no assumption, and it still sorts. + // https://github.com/asc-community/AngouriMath/issues/887 + [InlineData("x + a + b + c + arctan(x2) + d + e + 1/2 - 23 * sqrt(3) + arccot(x * x)", "x + a + b + c + arctan(x ^ 2) + d + e + 1/2 - 23 * sqrt(3) + arccotan(x ^ 2)")] [InlineData("a / b + c + d + e + f + sin(x) + arcsin(x) + 1 + 0 - a * (b ^ -1)", "1 + arcsin(x) + sin(x) + c + d + e + f provided not b = 0")] // Skipped // [InlineData("sin(arcsin(c x) + arccos(x c) + c)2 + a + b + sin(x) + 0 + cos(c - -arcsin(c x) - -arccos(-c x * (-1)))2", "1")]