Add jsnums-test spec for atan2 quadrant correctness#1866
Merged
Conversation
Joe says: Claude helped write some tests for the cases of atan2 that are likely to short-circuit and produce some of the fixnum errors/roughnum-vs-exact-int errors that check-pyret-number was designed for. Builds on #1864, this can rebsae onto horizon once that's merged Claude says: Adds an `it("atan2 four quadrants and boundaries")` block that pins down atan2's return-type contract across every combination of axis sign and quadrant. atan2's x<0 and (x>0, y<0) branches historically leaked bare Math.PI / 2*Math.PI through `add`, because JS-number+JS-number takes a fast path that returns the raw sum unchanged — so atan2(0, -1) returned the JS double 3.141592653589793 rather than a Roughnum, with branch- dependent type tags visible to user code. The library fix landed recently; this spec captures the expected behavior so future regressions trip CI rather than reappearing as branch-by-branch mysteries. Uses `isPyretNumber` rather than `isRoughnum` as the predicate because atan(0) currently short-circuits to an integer fixnum (not a Roughnum), so cases like atan2(0, 1) come back as 0 rather than ~0. That's a separate trig-type-consistency wart and is called out in a comment. Pins atan2(-1, 0) at 3π/2 explicitly: jsnums uses [0, 2π) for the negative-y axis, unlike JS Math.atan2 which would give -π/2 here. If we ever decide to switch conventions, this assertion makes the choice explicit instead of silently regressing user code. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Member
|
To be clear, I'm pretty sure we wanted atan(0) to be exact integer zero, so that math teachers can't claim that "we can't even get that simple case exactly right" or some argument like that... |
Joe says: Leaving in both isInteger and the toBe(0) because of what Claude said, but also because this test is coming right after a regression/update around isInteger and isPyretNumber, so it's useful to check that Claude says: - atan2(0, x>0) now asserts `isInteger` + `equals(_, 0)`, covering small int, larger int, and bignum x. `equals`/`isInteger` (rather than `===`/`toBe(0)`) keeps the spec at the "exact integer with value 0" level instead of pinning the JS-number representation. - Every other case is provably irrational (multiples of π) and now asserts `isRoughnum` directly. `make jsnums-test` passes. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
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
2 participants
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.
Joe says:
Claude helped write some tests for the cases of
atan2that are likely to short-circuit and produce some of the fixnum errors/roughnum-vs-exact-int errors that check-pyret-number was designed for.(Probably didn't need Claude for this TBH)
Builds on #1864, this can retarget onto horizon once that's merged
Claude says:
Adds an
it("atan2 four quadrants and boundaries")block that pins down atan2's return-type contract across every combination of axis sign and quadrant. atan2's x<0 and (x>0, y<0) branches historically leaked bare Math.PI / 2*Math.PI throughadd, because JS-number+JS-number takes a fast path that returns the raw sum unchanged — so atan2(0, -1) returned the JS double 3.141592653589793 rather than a Roughnum, with branch- dependent type tags visible to user code. The library fix landed recently; this spec captures the expected behavior so future regressions trip CI rather than reappearing as branch-by-branch mysteries.Uses
isPyretNumberrather thanisRoughnumas the predicate because atan(0) currently short-circuits to an integer fixnum (not a Roughnum), so cases like atan2(0, 1) come back as 0 rather than ~0. That's a separate trig-type-consistency wart and is called out in a comment.Pins atan2(-1, 0) at 3π/2 explicitly: jsnums uses [0, 2π) for the negative-y axis, unlike JS Math.atan2 which would give -π/2 here. If we ever decide to switch conventions, this assertion makes the choice explicit instead of silently regressing user code.