diff --git a/BREAKING-CHANGES.md b/BREAKING-CHANGES.md index a3a9eb0c5..c43713824 100644 --- a/BREAKING-CHANGES.md +++ b/BREAKING-CHANGES.md @@ -64,6 +64,7 @@ read first. | **silent** | a `RewriteRecording` across an `await`, or work started under it | lost, or somebody else's | follows the call | | 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 | --- @@ -193,6 +194,52 @@ The pairs elsewhere in the API are unaffected, because none of them has to guess integration `Range`, the arguments of `Substitute`, the cases of `MathS.Piecewise` and `ToProvided` are all ordered pairs whose two halves have distinct, stated roles. +### An inverse trigonometric function no longer cancels its own function off the principal branch + +`arcsin` is a left inverse of `sin` only on `[-pi/2, pi/2]`, and the three siblings likewise only on +their own intervals. The rewrite was unconditional, so `Simplify` returned a value that is wrong at +every real point outside the interval: + +| | was | is | the value at that point | +|---|---|---|---| +| `arcsin(sin(x))` | `x` | left as written | `pi - x` for `x` in `[pi/2, 3pi/2]`, and so on | +| `arccos(cos(x))` | `x` | left as written | `2*pi - x` for `x` in `[pi, 2pi]` | +| `arctan(tan(x))` | `x` | left as written | `x - pi` for `x` in `(pi/2, 3pi/2)` | +| `arccotan(cotan(x))` | `x` | left as written | `x - pi` for `x` in `(pi, 2pi)` | + +Measured on a build of `35f4ae5a` before this change and after it. The four rules are older than +1.4.0 and untouched since, so 1.4.0 answers the same way, but the numbers below are from the two +2.0.0 builds rather than from a 1.4.0 one: + +``` +"arcsin(sin(x))".Simplify().Substitute("x", 3).EvalNumerical() + was 3 + is 0.14159... which is pi - 3, and is what arcsin(sin(3)) equals + +"arctan(tan(x))".Simplify().Substitute("x", 2).EvalNumerical() + was 2 + is -1.14159... = 2 - pi + +"arccos(cos(x))".Simplify().Substitute("x", 4).EvalNumerical() + was 4 + is 2.28318... = 2*pi - 4 +``` + +**Where the argument is a real number inside the interval, the cancellation still happens and stays +exact**: `arcsin(sin(1/2))` is `1/2`, not a decimal. Where it is symbolic, the expression is left as +written, which is what SymPy and Mathematica both answer. + +A condition was deliberately *not* attached. `arcsin(sin(x))` is defined for every real `x`, so +`x provided x >= -pi/2 and x <= pi/2` would say the expression is undefined outside the interval +when in fact it merely has another value — trading a wrong value for a wrong domain. + +**The other direction is unchanged**, and needs no assumption: `sin(arcsin(z))`, `cos(arccos(z))`, +`tan(arctan(z))` and `cotan(arccotan(z))` are all `z`, because they compose the *right* inverse. + +`CircleTest.Test8` asserted `arccotan(cotan(3x)) == 3x` and was pinning the wrong answer; it now +asserts that the expression is left alone. Issue +[#884](https://github.com/asc-community/AngouriMath/issues/884). + ### 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 922b4a769..e0dbe5126 100644 --- a/Sources/AngouriMath/Functions/Simplification/Patterns/Patterns.Trigonometry.cs +++ b/Sources/AngouriMath/Functions/Simplification/Patterns/Patterns.Trigonometry.cs @@ -6,12 +6,55 @@ // using System; +using PeterO.Numbers; using static AngouriMath.Entity; namespace AngouriMath.Functions { internal static partial class Patterns { + /// The real value of where there is one to read. A rule + /// that holds only on a principal branch may fire only where the branch can be + /// decided, so a symbolic argument -- or one off the real line -- has to leave the + /// node alone. + private static bool TryReadReal(Entity argument, out EDecimal value) + { + value = EDecimal.Zero; + if (argument.Evaled is not Number.Real real || !real.EDecimal.IsFinite) return false; + value = real.EDecimal; + return true; + } + + /// Whether the argument lies in [-pi/2, pi/2], or in the open interval + /// when is false — the intervals arcsin and + /// arctan answer in. + /// + /// Tested by doubling the argument and comparing against pi rather than by halving pi, + /// because addition of two s is exact and division is not. The + /// comparison is still only as good as the working precision, and an argument that + /// close to an endpoint is treated as outside the interval — which declines to + /// rewrite, the safe direction. + /// + private static bool WithinHalfPi(Entity argument, bool closed) + { + if (!TryReadReal(argument, out var value)) return false; + var absolute = value.Abs(); + var comparison = absolute.Add(absolute).CompareTo(MathS.DecimalConst.pi); + return closed ? comparison <= 0 : comparison < 0; + } + + /// Whether the argument lies in [0, pi], or in the open interval when + /// is false — the intervals arccos and + /// arccotan answer in. + private static bool WithinZeroAndPi(Entity argument, bool closed) + { + if (!TryReadReal(argument, out var value)) return false; + if (value.IsNegative || (!closed && value.IsZero)) return false; + var comparison = value.CompareTo(MathS.DecimalConst.pi); + return closed ? comparison <= 0 : comparison < 0; + } + + internal static Entity TrigonometricRules(Entity x) => x switch { // sin({}) * cos({}) = 1/2 * sin(2{}) @@ -58,13 +101,27 @@ internal static partial class Patterns Mulf(Tanf(var any1), Cotanf(var any1a)) when any1 == any1a => 1, Mulf(Cotanf(var any1), Tanf(var any1a)) when any1 == any1a => 1, - // arcfunc(func(x)) = x - Arcsinf(Sinf(var any1)) => any1, - Arccosf(Cosf(var any1)) => any1, - Arctanf(Tanf(var any1)) => any1, - Arccotanf(Cotanf(var any1)) => any1, - - // func(arcfunc(x)) = x + // arcfunc(func(x)) = x, but only where x already lies in the interval arcfunc + // answers in. Outside it the composition folds back into that interval, so + // arcsin(sin(3)) is pi - 3 and arccos(cos(4)) is 2*pi - 4. Applied + // unconditionally this returned a wrong value at ordinary real points -- + // https://github.com/asc-community/AngouriMath/issues/884 + // + // A symbolic argument is left as written, which is what SymPy and Mathematica + // both do, and is a legitimate answer where returning x is not. Attaching the + // interval as a condition instead would be a second wrong answer: the + // composition is defined for every real x, so saying it is undefined outside the + // interval trades a wrong value for a wrong domain. + // + // The two tangent-family intervals are open, because tan and cotan have no value + // at the endpoints and the composition has none there either. + 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, + + // 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. Sinf(Arcsinf(var any1)) => any1, Cosf(Arccosf(var any1)) => any1, Tanf(Arctanf(var any1)) => any1, diff --git a/Sources/Tests/UnitTests/Common/CircleTest.cs b/Sources/Tests/UnitTests/Common/CircleTest.cs index 200409baf..64b96eeaf 100644 --- a/Sources/Tests/UnitTests/Common/CircleTest.cs +++ b/Sources/Tests/UnitTests/Common/CircleTest.cs @@ -85,8 +85,14 @@ public void DecimalToRational(string @decimal, string rational) [Fact] public void Test7() => Assert.Equal(3 * x, MathS.Sin(MathS.Arcsin(x * 3)).Simplify()); + // This asserted 3 * x, which is not what arccotan(cotan(3x)) is: arccotan answers in + // (0, pi), so at x = 2 the expression is about 2.86 and 3 * x is 6. The test was + // pinning a wrong answer, and the rewrite behind it now fires only where the argument + // is a real number inside that interval -- + // https://github.com/asc-community/AngouriMath/issues/884 [Fact] - public void Test8() => Assert.Equal(3 * x, MathS.Arccotan(MathS.Cotan(x * 3)).Simplify()); + public void Test8() => Assert.Equal(MathS.Arccotan(MathS.Cotan(3 * x)), + MathS.Arccotan(MathS.Cotan(x * 3)).Simplify()); [Theory] [InlineData("x / y + x * x * y")] diff --git a/Sources/Tests/UnitTests/Common/SimplificationRegressionTest.cs b/Sources/Tests/UnitTests/Common/SimplificationRegressionTest.cs index 5c9753399..01192428a 100644 --- a/Sources/Tests/UnitTests/Common/SimplificationRegressionTest.cs +++ b/Sources/Tests/UnitTests/Common/SimplificationRegressionTest.cs @@ -521,5 +521,69 @@ public void AContradictoryPairOfComparisonsFailsOverTheReals(string input) using var _ = MathS.Settings.Codomain.Set(Domain.Real); Assert.Equal(Entity.Boolean.False, input.ToEntity().Simplify()); } + + // https://github.com/asc-community/AngouriMath/issues/884 + // arcsin is a left inverse of sin only on [-pi/2, pi/2], and the three siblings + // likewise only on their principal intervals. The rewrite was unconditional, so + // arcsin(sin(3)) simplified to 3 where the value is pi - 3 -- a wrong answer at an + // ordinary real point, not a missing one. + // + // Asserted numerically because the point of the bug is the value: comparing the + // simplified form against the original at a point outside the principal interval + // is the only assertion that fails for the right reason. + [Theory] + [InlineData("arcsin(sin(x))", "3")] + [InlineData("arcsin(sin(x))", "pi")] + [InlineData("arcsin(sin(x))", "-2")] + [InlineData("arccos(cos(x))", "4")] + [InlineData("arccos(cos(x))", "-1")] + [InlineData("arctan(tan(x))", "2")] + [InlineData("arctan(tan(x))", "-2")] + [InlineData("arccotan(cotan(x))", "-1")] + public void SimplifyingAnInverseOfItsOwnFunctionKeepsTheValueOffThePrincipalBranch( + string expression, string at) + { + var original = expression.ToEntity(); + var simplified = original.Simplify(); + var before = original.Substitute("x", at.ToEntity()).EvalNumerical(); + var after = simplified.Substitute("x", at.ToEntity()).EvalNumerical(); + Assert.True(Magnitude(before - after) < 1e-20, + $"{expression} simplified to {simplified.Stringize()}, which at x = {at} is " + + $"{after.Stringize()} rather than {before.Stringize()}"); + } + + // The cancellation is still wanted where the argument is inside the principal + // interval, and there it has to stay exact rather than becoming a decimal. The + // expectation is simplified too because "1/2" parses as a Divf rather than a + // Rational (https://github.com/asc-community/AngouriMath/issues/873), and Entity + // equality is structural; a decimal answer still fails, which is what this pins. + [Theory] + [InlineData("arcsin(sin(1/2))", "1/2")] + [InlineData("arccos(cos(1/2))", "1/2")] + [InlineData("arctan(tan(1/2))", "1/2")] + [InlineData("arcsin(sin(0))", "0")] + [InlineData("arctan(tan(-1/3))", "-1/3")] + public void SimplifyingAnInverseOfItsOwnFunctionStaysExactOnThePrincipalBranch( + string expression, string expected) + { + Assert.Equal(expected.ToEntity().Simplify(), expression.ToEntity().Simplify()); + } + + // The other direction composes the *right* inverse and needs no assumption at all: + // sin(arcsin(z)) is z wherever arcsin(z) is defined. Pinned so that guarding the + // unsound half does not take the sound half with it. + [Theory] + [InlineData("sin(arcsin(x))")] + [InlineData("cos(arccos(x))")] + [InlineData("tan(arctan(x))")] + [InlineData("cotan(arccotan(x))")] + public void ComposingAFunctionOverItsOwnInverseIsStillTheIdentity(string input) + { + Assert.Equal("x".ToEntity(), input.ToEntity().Simplify()); + } + + static double Magnitude(Entity difference) => + ((System.Numerics.Complex)difference.EvalNumerical()).Magnitude; + } }