From 6f79d8cc012808d3987fd30db87ef3367ed70d24 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 22 Sep 2026 20:44:00 +0000 Subject: [PATCH] Refuse a sine whose reduction needs more of pi than exists [patch] PiTo serves at most ConstantPrecision digits and returns the capped constant rather than failing. That is deliberate, documented on the accessor and pinned by a test, so it is not the defect. The defect is that SinCos asked for more and assumed it got it. Reducing modulo pi/2 cancels the argument's integer digits out of pi, so the constant has to carry those on top of the answer. Past 150 it could not, and the result still reported the full significantDigits, which on this type is a promise that those digits are correct. Measured against references computed independently at several hundred digits, the correct digits come out as min(significantDigits, ConstantPrecision - integerDigits): an argument of 80 integer digits yields 71 correct digits whether 100 or 140 are asked for, one of 50 yields 100, and Sin(1e200) has no correct digits at any precision. RequireReducibleArgument enforces that sum. Deliberately not the wider reductionDigits the call site passes to PiTo: that pads the requirement with TrigonometricGuardDigits twice over as margin, and margin is allowed to be unavailable. Bounding it would refuse Sin(1000000, 130), which asks pi for 157 digits, is served 150, and is correct to every digit it reports -- as TestSinReducesALargeArgumentAgainstAWidePi has always pinned against an independent reference. The same measurement clears the rest of the surface the issue lists. Log is unaffected, carrying magnitude in the decimal exponent rather than cancelling it, and was correct to 130 digits at 180 total working digits. Exp tops out around ConstantPrecision - 1, inside the one-digit noise of the measurement everywhere short of that, so it is documented rather than guarded on this evidence. Fixes #96 Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01XH3MikiXVCii3abMojvibs --- CLAUDE.md | 1 + .../PreciseNumberTrigonometryTests.cs | 56 ++++++++++++ PreciseNumber/PreciseNumber.Trigonometry.cs | 85 ++++++++++++++++++- 3 files changed, 139 insertions(+), 3 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 6ab5460..2a8295d 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -42,6 +42,7 @@ dotnet run -c Release --project PreciseNumber.Benchmarks -- --filter '*' --job s - A fractional `Pow` is `exp(y · ln x)`, carried wider by the integer digits of `y · ln x` because `Exp`'s range reduction consumes them. The integer path stays exponentiation by squaring and is exact; tests pin that exactness rather than a tolerance - The `sanitize` constructor parameter controls whether trailing zeros are removed (default: true) - Constants (`Zero`, `One`, `Pi`, `E`, `Tau`) are pre-computed static instances +- `ConstantPrecision` is a ceiling, not just a width. `PiTo`/`ETo`/`Ln2To`/`Ln10To` serve at most that many digits and return the capped constant rather than failing, which is deliberate and pinned by `TestConstantAccessorsReturnTheWholeConstantWhenAskedForMore` — so a caller that *needs* what it asked for has to check, because the accessor will not. The circular functions are where this bites: reducing modulo `π/2` spends one digit of π per integer digit of the argument, so only about `ConstantPrecision - integerDigits` are left for the answer. `RequireReducibleArgument` enforces exactly that sum and nothing wider. Do not tighten it to `reductionDigits`: that pads the requirement with `TrigonometricGuardDigits` twice over as margin, margin is allowed to be unavailable, and bounding it would refuse `Sin(1000000, 130)` — a call whose every reported digit is correct. The half-turn family (`SinPi` and the rest) has no such ceiling, because it reduces before it multiplies, and `Log` has none either, because it carries the magnitude in the decimal exponent rather than cancelling it against a constant - As a value type it can't be null or inherited. Don't add null checks for `PreciseNumber` parameters, and don't reintroduce `protected` members - Conversions to integer types go through `BigInteger`, so range checks, clamping, and wrapping follow its conventions. Conversions to `double`, `float`, `Half`, and `decimal` render normalized scientific notation (`d.ddd…E±n`) and parse it, because the runtime parsers round correctly, with Clinger's fast path for small values. Keep one digit before the point. The .NET 7 and 8 parsers clamp an exponent above 1000 and still offset it by every digit ahead of the point, so a long significand rendered as an integer parses as zero there. Conversions from `double`, `float`, and `Half` use the shortest text that round-trips (`"R"`). NaN and infinity coming in follow `BigInteger` too diff --git a/PreciseNumber.Test/PreciseNumberTrigonometryTests.cs b/PreciseNumber.Test/PreciseNumberTrigonometryTests.cs index ee4a9cc..cdba4d8 100644 --- a/PreciseNumber.Test/PreciseNumberTrigonometryTests.cs +++ b/PreciseNumber.Test/PreciseNumberTrigonometryTests.cs @@ -122,6 +122,62 @@ public void TestSinReducesALargeArgumentAgainstAWidePi() AssertAgreesTo(Parse(SinOneMillionDigits), sine, 120, "Sin(1000000) did not match its wide reference"); } + [TestMethod] + public void TestSinRefusesMoreDigitsThanPiCanReduceAgainst() + { + // The issue's acceptance criterion. Reducing modulo π/2 cancels the argument's integer + // digits, so π has to carry those on top of the answer. PiTo caps at ConstantPrecision and + // returns the capped constant rather than failing, so this used to come back reporting every + // digit asked for while only about ConstantPrecision - argumentDigits of them were real. + PreciseNumber million = Parse("1000000"); + + ArgumentOutOfRangeException thrown = Assert.ThrowsExactly( + () => PreciseNumber.Sin(million, PreciseNumber.ConstantPrecision - 6)); + + // The message has to name the shortfall, or a caller cannot tell what to ask for instead. + StringAssert.Contains(thrown.Message, "144 significant digits needs", StringComparison.Ordinal); + StringAssert.Contains(thrown.Message, "143 significant digits are available", StringComparison.Ordinal); + } + + [TestMethod] + public void TestSinDeliversEveryDigitUpToTheReductionCeiling() + { + // The other half, and the reason the bound is where it is rather than somewhere safer. One + // million has seven integer digits, so ConstantPrecision - 7 digits is the most π can + // support — and that request has to succeed and be correct, not be refused out of caution. + PreciseNumber sine = PreciseNumber.Sin(Parse("1000000"), PreciseNumber.ConstantPrecision - 7); + + Assert.AreEqual(PreciseNumber.ConstantPrecision - 7, sine.SignificantDigits); + AssertAgreesTo(Parse(SinOneMillionDigits), sine, 120, "Sin at the reduction ceiling did not match its wide reference"); + } + + [TestMethod] + public void TestSinRefusesAnArgumentTooLargeToReduceAtAll() + { + // Magnitude alone is enough, without asking for many digits. An argument of 201 integer + // digits cancels more of π than exists, so there is no precision at which the answer means + // anything — where before it returned digits that were simply invented. + PreciseNumber enormous = Parse("1E200"); + + Assert.ThrowsExactly(() => PreciseNumber.Sin(enormous, 10)); + Assert.ThrowsExactly(() => PreciseNumber.Cos(enormous, 10)); + Assert.ThrowsExactly(() => PreciseNumber.Tan(enormous, 10)); + } + + [TestMethod] + public void TestOrdinaryAnglesAreUnaffectedByTheReductionCeiling() + { + // The guard on the guard. Every angle in the sweep at a sane precision has to keep working, + // so the ceiling cannot be reached by ordinary use — only the combination of a large + // magnitude and a high precision is beyond what the constant can serve. + foreach (PreciseNumber angle in AngleSweep()) + { + (PreciseNumber sin, PreciseNumber cos) = PreciseNumber.SinCos(angle, 60); + Assert.IsTrue(sin.SignificantDigits <= 60, $"Sin({angle}) reported more digits than were asked for"); + Assert.IsTrue(cos.SignificantDigits <= 60, $"Cos({angle}) reported more digits than were asked for"); + } + } + [TestMethod] public void TestPythagoreanIdentityHoldsAcrossTheSweep() { diff --git a/PreciseNumber/PreciseNumber.Trigonometry.cs b/PreciseNumber/PreciseNumber.Trigonometry.cs index 1757f7c..db996e6 100644 --- a/PreciseNumber/PreciseNumber.Trigonometry.cs +++ b/PreciseNumber/PreciseNumber.Trigonometry.cs @@ -16,6 +16,13 @@ namespace ktsu.PreciseNumber; /// rather than the property — which is exactly why /// of a large angle is meaningful at all. /// +/// π is stored to digits, so that same d + n is a ceiling as +/// well as a requirement: about ConstantPrecision - d digits are available at magnitude +/// 10^d, and a circular function asked for more is refused rather than answered with digits +/// the constant never carried. The half-turn family below has no such ceiling, because it reduces +/// before it multiplies and so never needs a wide π at all. +/// +/// /// and reduce modulo π/2 /// into [-π/4, π/4] with an octant index, so one kernel serves both and the series stays /// short. exists to do that reduction once for a caller that @@ -100,7 +107,12 @@ public static PreciseNumber Sin(PreciseNumber x) => /// The angle, in radians. /// The number of significant digits to produce. /// The sine of . - /// Thrown when is less than one. + /// + /// Thrown when is less than one, or when it and the integer + /// digits of together exceed . Reducing the + /// argument cancels its integer digits out of π, so those and the answer both have to fit inside + /// the stored constant; see . + /// public static PreciseNumber Sin(PreciseNumber x, int significantDigits) => SinCos(x, significantDigits).Sin; @@ -123,7 +135,12 @@ public static PreciseNumber Cos(PreciseNumber x) => /// The angle, in radians. /// The number of significant digits to produce. /// The cosine of . - /// Thrown when is less than one. + /// + /// Thrown when is less than one, or when it and the integer + /// digits of together exceed . Reducing the + /// argument cancels its integer digits out of π, so those and the answer both have to fit inside + /// the stored constant; see . + /// public static PreciseNumber Cos(PreciseNumber x, int significantDigits) => SinCos(x, significantDigits).Cos; @@ -146,13 +163,22 @@ public static (PreciseNumber Sin, PreciseNumber Cos) SinCos(PreciseNumber x) => /// The angle, in radians. /// The number of significant digits to produce. /// A tuple of the sine and cosine of . - /// Thrown when is less than one. + /// + /// Thrown when is less than one, or when it and the integer + /// digits of together exceed . + /// /// /// x = q · π/2 + r with q the nearest integer and r in [-π/4, π/4]. /// The kernel evaluates the sine and cosine of r, and q mod four selects which, and /// with which sign, becomes the sine and cosine of x. The π/2 the reduction /// subtracts is read wide enough that the integer part it cancels was present in the constant /// rather than invented. + /// + /// That width is bounded: π is stored to digits, the reduction + /// spends one per integer digit of , and what is left is the most the answer + /// can carry. Beyond it the call is refused rather than answered with digits the constant never + /// held — see . + /// /// public static (PreciseNumber Sin, PreciseNumber Cos) SinCos(PreciseNumber x, int significantDigits) { @@ -170,6 +196,8 @@ public static (PreciseNumber Sin, PreciseNumber Cos) SinCos(PreciseNumber x, int int argumentDigits = IntegerDigitCount(x); int reductionDigits = working + argumentDigits + TrigonometricGuardDigits; + RequireReducibleArgument(significantDigits, argumentDigits); + PreciseNumber piOverTwo = Divide(PiTo(reductionDigits), Two, reductionDigits); BigInteger quadrant = RoundToNearestInteger(Divide(x, piOverTwo, reductionDigits)); PreciseNumber remainder = Subtract(x, Multiply(new(0, quadrant), piOverTwo)) @@ -869,4 +897,55 @@ private static PreciseNumber AtanSeries(PreciseNumber z, int workingDigits) throw new ArithmeticException( $"The arc tangent series did not converge to {workingDigits.ToString(InvariantCulture)} significant digits."); } + + /// + /// Throws when reducing an argument modulo π/2 would need more of π than + /// carries. + /// + /// The significant digits the caller asked for. + /// The integer digits of the argument, which the reduction cancels. + /// + /// Thrown when the two together exceed . + /// + /// + /// + /// serves at most digits and says so, but + /// it returns the capped constant rather than failing, so a reduction that asked for more had no + /// way to know it had been short-changed. The result still reported the full + /// , which on this type is a promise that those digits are + /// correct. + /// + /// + /// The bound is the answer's width plus the digits the reduction destroys, not the padded + /// reductionDigits the caller passes to . Those carry + /// twice over as margin, and margin is allowed to be + /// unavailable: Sin(1000000, 130) asks π for 157 digits, is served 150, and is still + /// correct to every digit it reports, which + /// TestSinReducesALargeArgumentAgainstAWidePi pins against an independent reference. + /// Bounding reductionDigits instead would refuse that call. + /// + /// + /// Measured rather than assumed. Against references computed independently at several hundred + /// digits, the correct digits of a sine come out as + /// min(significantDigits, ConstantPrecision - argumentDigits): an argument of 80 integer + /// digits yields 71 correct digits whether 100 or 140 are requested, and one of 50 integer digits + /// yields 100. The sum crossing is exactly where the requested + /// digits stop being delivered. + /// + /// + private static void RequireReducibleArgument(int significantDigits, int argumentDigits) + { + if (significantDigits + argumentDigits > ConstantPrecision) + { + int available = Math.Max(0, ConstantPrecision - argumentDigits); + throw new ArgumentOutOfRangeException( + nameof(significantDigits), + significantDigits, + $"Reducing an argument of {argumentDigits.ToString(InvariantCulture)} integer digits to " + + $"{significantDigits.ToString(InvariantCulture)} significant digits needs π to " + + $"{(significantDigits + argumentDigits).ToString(InvariantCulture)} digits, but it is stored to " + + $"{ConstantPrecision.ToString(InvariantCulture)}. At most " + + $"{available.ToString(InvariantCulture)} significant digits are available at this magnitude."); + } + } }