Read arccotan's range off arccotan rather than off a textbook (#887)

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

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;
         }
 
+        /// <summary>Whether the argument lies in the range <c>arccotan</c> answers in, which is
+        /// <c>(-pi/2, pi/2]</c> without zero.</summary>
+        /// <remarks>
+        /// This library's <c>arccotan</c> is <c>arctan(1/x)</c> extended by
+        /// <c>arccotan(0) = pi/2</c>, so its range is that half-open interval and **not** the
+        /// <c>(0, pi)</c> that many texts use — <c>arccotan(-1)</c> is <c>-pi/4</c>. Zero is
+        /// excluded because <c>cotan</c> has no value there, so the composition has none either
+        /// and rewriting it to <c>x</c> would invent one.
+        /// https://github.com/asc-community/AngouriMath/issues/887
+        /// </remarks>
+        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;
+        }
+
+        /// <summary>
+        /// <c>arctan(x) + arccotan(x)</c>, which is <c>pi/2</c> where <c>x</c> is a non-negative
+        /// real and <c>-pi/2</c> where it is negative, or <see langword="null"/> where the sign
+        /// cannot be read.
+        /// </summary>
+        /// <remarks>
+        /// It follows from the range above: for positive <c>x</c> both terms are positive and sum
+        /// to <c>pi/2</c>, and for negative <c>x</c> both are negative. <c>pi/2 * sgn(x)</c> is
+        /// the closed form and is wrong at exactly one point — <c>x = 0</c>, where the sum is
+        /// <c>pi/2</c> while <c>sgn</c> is <c>0</c> — so the sign is decided here rather than
+        /// written into the answer. A symbolic argument has no decidable sign and is left alone.
+        /// </remarks>
+        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;
+        }
+
         /// <summary>Whether the argument lies in <c>[0, pi]</c>, or in the open interval when
-        /// <paramref name="closed"/> is false — the intervals <c>arccos</c> and
-        /// <c>arccotan</c> answer in.</summary>
+        /// <paramref name="closed"/> is false — the interval <c>arccos</c> answers in.</summary>
         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.

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) =>

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

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);

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")]