Trigonometric describes the rules it runs, and its thirty-three identities are written out - #1114
Merged
Conversation
…ities are written out 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
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.
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 measured rather than recalled, and one found a wrong answer
because this library's
arccotanhas range(-pi/2, pi/2]and not the textbook(0, pi). Measuring that at +1, −1 and 0 rather than recalling it confirmed the identity — and showedSimplifycontradictingEvalNumericalonarccotan(-1)two lines up the same output. That is #1113, fixed separately.written that way deliberately, naming no interval. The interval is the thing this codebase has shipped wrong before, and the rule does not need it stated in order to be described.
Forty-three arms become thirty-three rules
RewriteRules.Trigonometric.Rules.CountThree tests changed, and each was asserting something the repoint improves
StepAsASentenceTestusedTrigonometricas 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 — the failure cost was the argument for asking instead of naming.RewriteRecordingTest.AOneStepRewriteIsOneStepasserted theswitcharm's replacement text ("1") forsin²+cos². It now asserts the rule's name and identity, which is what a derivation reports — and its two cases now show the contrast directly:CommonnamedSumf(var any1, var any1a) when any1 == any1a,Trigonometricnameda-squared-sine-and-cosine-of-one-angle-sum-to-onewithsin(a)^2 + cos(a)^2 = 1.DerivationPathTest.EveryStepNamesWhatDidItasserted a step's tier equals its set's. It is the weakest of the rewrites that actually fired (A step renders as a sentence #1110), andsin²+cos²is nowSoundwhereTrigonometricisSoundUnderAssumptions— the Pythagorean identity holds for every complex argument, and the set's minimum was understating it. That is the finer grain doing its job, so the test now asserts the documented relation rather than the old one.What is left
Twenty-four of the thirty sets describe what they run. Three of the six left are the
CanonicalOrderfamily, which still runs itsswitch. The three others areCommon(33 descriptions to port),Power(22) andInequalityEquality(11).Full suite: 9010 passed, 14 skipped, 0 failed. No public API change.
Part of #746 tier 2 and #825.