From b08fc5d25eb50952ad86c66a19b89fdb01261d0b Mon Sep 17 00:00:00 2001 From: Rafael Vuijk Date: Tue, 11 Aug 2026 13:38:14 +0000 Subject: [PATCH] Read arccotan's range off arccotan rather than off a textbook (#887) This library's arccotan is arctan(1/x) extended by arccotan(0) = pi/2, so its range is (-pi/2, pi/2] and not the (0, pi) many texts use: arccotan(-1) is -pi/4. Two rewrites assumed otherwise. arctan(x) + arccotan(x) was pi/2 unconditionally, which is a wrong answer at every negative real -- the sum is -pi/2 there. pi/2 * sgn(x) is the closed form and is wrong at exactly one point, x = 0, where the sum is pi/2 and sgn is 0, so the sign is decided where it can be read and the sum left alone otherwise. A Piecewise would be total, but Compile throws on one, so answering that way would stop expressions compiling that compile today. The neighbouring arcsin(x) + arccos(x) is unconditional and correct, because arccos(x) is pi/2 - arcsin(x) by definition over the whole plane; the block had a sound half and an unsound half again. arccotan(cotan(x)) was guarded with [0, pi] in #885, by me, on the same wrong assumption about the range. That admitted (pi/2, pi), where the rewrite is false -- arccotan(cotan(2)) simplified to 2 and is 2 - pi -- 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 #885 check out against the same measurements. Three tests moved, each pinning the old wrong answer for a symbolic argument, and each now covering both signs with numbers. SortSimplifyTest's case sorted its whole sum only because the collapse shortened it; with the collapse gone the sum stays, and the sibling arcsin case still collapses and still sorts. Found by work/boundcheck on its first run, which is the point of it: the shapes come from the nodes by reflection and the points are chosen where an assumption fails, so it found an error in my own merged fix within minutes of existing. Measured on this branch: 6296 C# tests pass, 130 F# tests pass, casbench 117/119 with 0 wrong, propcheck 1340 checks with 0 failures, rootcheck 596/596, simpsweep 10463/10463 agreeing, boundcheck 5 disagreements down to 4 -- the rest being #884's logarithm half and two findings filed on #887. --- BREAKING-CHANGES.md | 42 +++++++++++ .../Patterns/Patterns.Trigonometry.cs | 57 +++++++++++++-- .../UnitTests/Common/ArctanIdentitiesTest.cs | 9 ++- .../Common/SimplificationRegressionTest.cs | 70 +++++++++++++++++++ .../UnitTests/PatternsTest/SimplifyTest.cs | 11 ++- .../PatternsTest/SortSimplifyTest.cs | 9 ++- 6 files changed, 189 insertions(+), 9 deletions(-) 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")]