Refuse a sine whose reduction needs more of π than exists [patch] - #97
Merged
matt-edmondson merged 1 commit intoSep 23, 2026
Merged
Conversation
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 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XH3MikiXVCii3abMojvibs
|
matt-edmondson
deleted the
claude/precisenumber-96-constant-precision-ceiling
branch
September 23, 2026 00:04
This was referenced Sep 23, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.



Fixes #96
The fix is not where the issue points, and the surface is narrower
The issue proposes throwing from
ConstantTowhen more digits are asked for than the constants hold. I did that first, and it broke two existing tests — both correctly. The capping is not the defect:ConstantTo's behaviour is documented on every accessor ("orPiitself when that is no fewer digits than it carries") and pinned byTestConstantAccessorsReturnTheWholeConstantWhenAskedForMore, which assertsPiTo(int.MaxValue) == Pi.PiTo(n)means at mostndigits, by design.TestSinReducesALargeArgumentAgainstAWidePi. That test callsSin(1000000, 130), which asks π for 157 digits and is served 150 — and its answer is correct to every digit it reports, against a committed independent reference. A blanket throw refuses a call that works.So the defect is the issue's second suggestion: the reduction call site asked for more and assumed it got it. And the guard has to be on what the reduction genuinely needs, not on the padded width it requests.
Measured, not assumed
I compared against references computed independently at several hundred digits, by two methods that share nothing with this library (mpmath, and a Taylor series in Python's
decimalover a hand-verified 200-digit π — the two agree to every digit compared).The correct digits of a sine come out as
min(significantDigits, ConstantPrecision − integerDigits):An argument of 80 integer digits yields 71 correct digits whether 100 or 140 are requested.
Sin(1e200)has no correct digits at any precision. The boundary is exactly wheresignificantDigits + integerDigitscrossesConstantPrecision.RequireReducibleArgumentenforces that sum. Deliberately notreductionDigits, which isworking + argumentDigits + TrigonometricGuardDigits— that carries the guard width twice over as margin, and margin is allowed to be unavailable. Bounding it would refuseSin(1000000, 130). The bound is conservative by at most one digit at the very edge, which is the right direction for a guard.The rest of the surface the issue lists
The same measurement clears most of it, which is why this PR is smaller than the issue implies:
Logis unaffected. It carries magnitude in the decimal exponent rather than cancelling it against a constant. Correct to 130 digits at 180 total working digits — no degradation at all.Exptops out aroundConstantPrecision − 1, and everywhere short of that it is inside the one-digit noise of the measurement. Documented rather than guarded: I do not have evidence sharp enough to place a threshold, and a guard on the wrong side of it would refuse working calls, which is the failure this PR exists to avoid.SinPiand the rest) has no ceiling by construction — it reduces modulo two before multiplying by π, as the class remarks already say.Tests
Four cases. The two
Refusesones are the issue's acceptance criterion.TestSinRefusesMoreDigitsThanPiCanReduceAgainstTestSinDeliversEveryDigitUpToTheReductionCeilingConstantPrecision - 7digits ofSin(1000000)succeeds and matches the wide reference — the bound is not merely safe, it is tightTestSinRefusesAnArgumentTooLargeToReduceAtAllSin/Cos/Tanof1e200at only 10 digits, where magnitude alone exhausts πTestOrdinaryAnglesAreUnaffectedByTheReductionCeilingThe message is asserted, not just the type, because "ask for fewer digits" is useless advice without a number.
Confirmed the tests depend on the change by neutering the magnitude term and re-running: 2 failed, 22 passed — exactly the two refusal cases. The other two pass either way by design; they are the guard against the guard reaching calls that were always valid, not evidence for it.
Full suite green: 354 total, 0 failed (350 before, 4 added), all four TFMs. Release build clean, zero warnings.
TestSinReducesALargeArgumentAgainstAWidePiis among the passing ones, which is the check that matters here.Not covered
ConstantTois unchanged, soPiTo(200)still returns 150 digits silently. That is its documented contract and a test pins it; changing it is a breaking API change that this bug does not require, now that the caller which actually depended on the missing digits checks for itself. If you would rather it throw, that is a separate decision and a[major].Atan2,Asin,AcosandDegreesToRadiansreadPiTo(working)without a magnitude term, so they degrade only past ~150 requested digits, the same mild case asExp. Not guarded here for the same reason.🤖 Generated with Claude Code
https://claude.ai/code/session_01XH3MikiXVCii3abMojvibs
Generated by Claude Code