A negative arccotan is negative - #1113
Merged
Merged
Conversation
`arccotan` here is `arctan(1/x)`, with range `(-pi/2, pi/2]` -- not the textbook `(0, pi)`.
`InverseTrigonometricTableValues.PullArccotan` read it as the textbook `pi/2 - arctan(x)`.
The two agree on every positive argument and on nothing negative, so a closed form came back
that was not the value:
"arccotan(-1)".ToEntity().EvalNumerical() // -0.7853981633974483..., which is -pi/4
"arccotan(-1)".ToEntity().Simplify() // 3/4 * pi
`Simplify` and `EvalNumerical` disagreeing about a constant is the sharpest form this kind of
defect takes. It reached every table value with a negative argument: `arccotan(-sqrt(3))` was
`5/6 * pi` and is `-1/6 * pi`; `arccotan(-(2 + sqrt(3)))` was `11/12 * pi` and is `-1/12 * pi`.
**The rule for `arctan(x) + arccotan(x)` had the convention right all along**, answering `pi/2`
for a non-negative argument and `-pi/2` for a negative one (#887). The table's docstring
claimed to take "the same reading of it the simplification rule for arctan(x) + arccotan(x)
already takes" and took the opposite one -- and the comment asserting they agreed is what let
the disagreement stand for as long as it did. A docstring is not a check.
The fix takes the sign from the *argument* rather than from the angle, because `arctan(0)` is
`0` and carries none: `arccotan(0)` is `pi/2` and not `-pi/2`.
`arccos` uses the same complement helper and is **not** affected -- its range is `[0, pi]` and
`arcsin`'s is `[-pi/2, pi/2]`, so `pi/2 - arcsin(x)` holds for every argument. The helper keeps
that reading and now says why on itself, and `ArccotanTableSignTest` asserts arccos is
unmoved so that a later tidy-up cannot merge the two paths back together.
**How it was found, which is the part worth keeping.** Writing an identity for the
`arctan + arccotan` rule during the rule-description work, and measuring the function at a
positive argument, a negative one and zero before writing the interval down rather than
recalling it. The measurement confirmed the identity and showed `Simplify` contradicting the
numeric value two lines further up the same output. The test measures the range the same way
rather than asserting it.
Recorded in BREAKING-CHANGES.md, both values measured on a build.
Full suite: 9010 passed, 14 skipped, 0 failed.
Rafael-SOWNet
added a commit
that referenced
this pull request
Aug 30, 2026
…ities are written out (#1114) The fourth tranche of the repoint. `Trigonometric`'s comments are notes on *soundness* -- which branch cut, which range, which issue -- rather than identities, so all thirty-three were written from the rules' own patterns and replacements. Two were checked by measurement rather than recalled, and one of those found a wrong answer elsewhere (fixed separately in #1113): - `arctan(a) + arccotan(a) = pi/2 for a >= 0, and -pi/2 for a < 0`, because this library's arccotan has range (-pi/2, pi/2] and not the textbook (0, pi); - `arccotan(cotan(a)) = a, for a in arccotan's own range` -- written that way deliberately, naming no interval, because the interval is the thing this codebase has shipped wrong before and the rule does not need it stated to be described. Forty-three arms become thirty-three rules. The registry total goes 407 -> 355 and described rules 95 -> 200, both measured. Three tests changed, and each was asserting something the repoint improves: - `StepAsASentenceTest` used Trigonometric as its example of a set still named by rendered patterns. It now *finds* those sets rather than naming one, since which they are moves with every tranche. - `RewriteRecordingTest.AOneStepRewriteIsOneStep` asserted the switch arm's replacement text for sin^2 + cos^2. It now asserts the rule's name and identity, which is what a derivation reports -- and the two cases now show the contrast directly, Common named by its rendered pattern against Trigonometric named in words. - `DerivationPathTest.EveryStepNamesWhatDidIt` asserted a step's tier equals its set's. It is the weakest of the rewrites that fired, per #1110, and sin^2 + cos^2 is now `Sound` where Trigonometric is `SoundUnderAssumptions` -- which is the finer grain doing its job. Twenty-four of the thirty sets now describe what they run. Three of the six left are the CanonicalOrder family, which still runs its switch. The three others are Common (33 descriptions to port), Power (22) and InequalityEquality (11). Full suite: 9010 passed, 14 skipped, 0 failed. No public API change. Part of #746 tier 2 and #825.
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
1 participant
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.
arccotanhere isarctan(1/x), with range(-pi/2, pi/2]— not the textbook(0, pi).InverseTrigonometricTableValues.PullArccotanread it as the textbookpi/2 - arctan(x). The two agree on every positive argument and on nothing negative, so a closed form came back that was not the value:SimplifyandEvalNumericaldisagreeing about a constant is the sharpest form this kind of defect takes. It reached every table value with a negative argument:arccotan(-1)3/4 * pi-1/4 * piarccotan(-sqrt(3))5/6 * pi-1/6 * piarccotan(-(2 + sqrt(3)))11/12 * pi-1/12 * piThe comment is what let it stand
The rule for
arctan(x) + arccotan(x)had the convention right all along, answeringpi/2for a non-negative argument and-pi/2for a negative one (#887).The table's docstring claimed to take "the same reading of it the simplification rule for arctan(x) + arccotan(x) already takes" — and took the opposite one. Two places in the library held opposite readings of the same function while one of them asserted they agreed. A docstring is not a check.
The fix takes the sign from the argument rather than from the angle, because
arctan(0)is0and carries none:arccotan(0)ispi/2and not-pi/2.arccos is not affected, and that is asserted
arccosuses the same complement helper and is untouched — its range is[0, pi]andarcsin's is[-pi/2, pi/2], sopi/2 - arcsin(x)holds for every argument. The helper keeps that reading and now says why on itself, andArccotanTableSignTestassertsarccosis unmoved so that a later tidy-up cannot merge the two paths back together.How it was found
Writing an identity for the
arctan + arccotanrule during the rule-description work (#1112), and measuring the function at a positive argument, a negative one and zero before writing the interval down rather than recalling it. The measurement confirmed the identity — and showedSimplifycontradicting the numeric value two lines further up the same output.ArccotanTableSignTestmeasures the range the same way rather than asserting it, which is the only thing that would have caught this and the only thing that will catch the next one.Recorded in
BREAKING-CHANGES.md, both values measured on a build.Full suite: 9010 passed, 14 skipped, 0 failed.