saloon: fix rtol sample mismatch in scalar transcendental dispatch#77
Open
sigilante wants to merge 1 commit into
Open
saloon: fix rtol sample mismatch in scalar transcendental dispatch#77sigilante wants to merge 1 commit into
sigilante wants to merge 1 commit into
Conversation
+sa's scalar helpers (+trans-scalar, +fun-scalar, +fadd/+fsub/+fmul/+fdiv/ +fabs/+fgte/+flte -- ~80 call sites total) called /lib/math's rh/rs/rd/rq doors as `~(fn rX:math [rnd rtol])`, but those doors take a 3-field sample `[r rtol atol]`, not 2. Since +sa's own `rtol` field is a bare `@r` atom (not a cell), this could never type-check at any precision -- confirmed by isolating the exact call pattern in a standalone probe and by testing the unmodified file directly: saloon.hoon failed `-build-file` on its own, before any of this change, at every one of rh/rs/rd/rq (the compiler just stops at the first failure in a `?-`, so only one bloq's error surfaced per attempt, making it look narrower than it is). Fixed by supplying an explicit, width-matched zero atol (`` `@rX`0 ``) at each call site, matching /lib/math's own documented "atol: absolute tolerance floor (default 0)". tests/lib/saloon-rays.hoon's existing suite only ever exercised bloq 5 (@rs) -- the type fix is mechanically identical across all four bloqs, but to actually prove it at runtime (not just "compiles"), this adds +close-rh/rd/rq + +test-exp-rh/rd/rq spot checks at the three previously- untested widths, cross-checked against ship-computed values. Also found, NOT fixed here (separate, unrelated issues; out of scope): - tests/lib/saloon.hoon (the pre-`saloon-rays.hoon` scalar test file) is stale, testing an `rs:saloon` API that no longer exists post-refactor -- saloon-rays.hoon's own header comment already documents this split. - tests/lib/saloon-eig.hoon and saloon-eigh.hoon (GENERATED by saloon/tools/eig_check.py) hit the exact same rtol-sample bug in their own hand-rolled `~(lth rd:math [%n .~1e-10])` sort comparator -- the fix belongs in the generator script, not the generated output. - tests/lib/saloon-unum.hoon has pre-existing 1-ULP mismatches at posit32 (tan/log/exp/cos), predating this fix: the test's expected values were captured before /lib/unum's Chebyshev-basis transcendental rewrite and were never refreshed. Verified: saloon-rays/-cplx/-eig-tol/-eigh-cc all green (`ok=%.y`). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
sigilante
added a commit
that referenced
this pull request
Jul 13, 2026
Adds a new +| %rand section to /lib/saloon's +sa core: +fill-uniform, +fill-normal, +fill-expon, +fill-below, filling a Lagoon $ray element-by- element in row-major (C) order (rand-spec.md section 8). Lands in Saloon itself, not a separate librand file -- the spec's framing describes each arm's shape, not a literal nested sub-core, and Saloon's own convention is one flat +sa door with +| section markers. %phil (Philox) elements get per-element counter treatment, the whole reason Philox is the primary engine: +fill-uniform (a single, non- rejecting draw per element) assigns element i counter ctr0+i directly, so the fill decomposes into n independent draws a future jet can parallelize. +fill-normal/+fill-expon/+fill-below are rejection-based, so they get a wider per-element counter WINDOW instead (ctr0 + i*2^32): the rejection loop walks freely within its own window, and the returned rng's counter is forced to ctr0+n*2^32 regardless of how many sub-draws each element actually used, so post-state is a pure function of n -- with an explicit crash if a single element's rejection loop ever walks past its own window (astronomically improbable, but checked rather than silently overflowing into the next element's window). Non-%phil engines (%sm64, %pcg) have no comparable jump primitive and aren't jet-parallelized anyway, so they just thread sequentially. %i754 only, bloq 5/6 (@rs/@rd) -- posit rays are deferred per the spec's stated v1 scope. This needed a pre-existing, unrelated bug in +sa's scalar transcendental dispatch fixed first (saloon.hoon didn't -build-file AT ALL beforehand, on any branch) -- shipped as its own PR (#77) since it's also needed upstream in urbit/urbit; that fix is duplicated here (not rebased) so +rand-ray has something to build against, to be reconciled once #77 merges to main. Hit two instances of the same Hoon footgun class writing +rand-ray itself (narrowing not surviving a recursive $(...) rebind): mutating .r inside a %phil-narrowed recursive trap lost the narrowing on re-entry (fixed by building fresh [%phil key0 ctrN] literals instead of r(ctr.p ...)); the non-%phil branch's .r is narrowed to EXCLUDE %phil, but +draw's return type is the full rng union, so the trap's first entry and its recursive re-entries disagreed on .r's type (fixed by explicitly widening `rng:rand`r once before the trap). tests/lib/saloon-rand-ray.hoon (in the saloon desk): every arm bit-exact cross-checked against a direct call to the underlying /lib/rand or /lib/i754rand primitive at the expected counter -- not just "doesn't crash" -- including the %phil per-element/window counter math and %sm64 sequential threading. Full existing saloon (saloon-rays/-cplx/-eig-tol/ -eigh-cc) and librand (rand/i754rand/twocrand/fixedrand/complexrand/ unumrand) regression suites reverified green. This completes milestone 8. Milestone 9 (final README/NEXT-STEPS pass) remains. Co-Authored-By: Claude Sonnet 5 <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
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.
Summary
+sa's scalar helpers (+trans-scalar,+fun-scalar,+fadd/+fsub/+fmul/+fdiv/+fabs/+fgte/+flte— ~80 call sites total) called/lib/math'srh/rs/rd/rqdoors as~(fn rX:math [rnd rtol]), but those doors take a 3-field sample[r rtol atol], not 2. Since+sa's ownrtolfield is a bare@ratom (not a cell), this could never type-check at any precision — confirmed by isolating the exact call pattern in a standalone probe and by testing the unmodified file directly:saloon.hoonfailed-build-fileon its own, before any change here, at every one ofrh/rs/rd/rq(the compiler stops at the first failure in a?-, so only one bloq's error surfaced per attempt, making it look narrower than it actually is).Fixed by supplying an explicit, width-matched zero
atol(`@rX`0) at each call site, matching/lib/math's own documented "atol: absolute tolerance floor (default 0)".tests/lib/saloon-rays.hoon's existing suite only ever exercised bloq 5 (@rs) — the type fix is mechanically identical across all four bloqs, but to actually prove it at runtime (not just "compiles"), this adds+close-rh/rd/rq++test-exp-rh/rd/rqspot checks at the three previously-untested widths, cross-checked against ship-computed values.Also found, NOT fixed here (separate, unrelated issues, flagged for a future pass):
tests/lib/saloon.hoon(the pre-saloon-rays.hoonscalar test file) is stale, testing anrs:saloonAPI that no longer exists post-refactor —saloon-rays.hoon's own header comment already documents this split.tests/lib/saloon-eig.hoonandsaloon-eigh.hoon(GENERATED bysaloon/tools/eig_check.py) hit the exact same rtol-sample bug in their own hand-rolled~(lth rd:math [%n .~1e-10])sort comparator — the fix belongs in the generator script, not the generated output.tests/lib/saloon-unum.hoonhas pre-existing 1-ULP mismatches at posit32 (tan/log/exp/cos), predating this fix: the test's expected values were captured before/lib/unum's Chebyshev-basis transcendental rewrite and were never refreshed.This surfaced while building librand milestone 8 (
+rand-rayin Saloon), which needed a workingsaloon.hoonto build against — filed as its own PR since the fix is also needed upstream inurbit/urbit.Test plan
-build-file %/lib/saloon/hoonsucceeds (previously failed on the unmodified file too).-test %/tests/lib/saloon-rays ~— all tests green (ok=%.y), including new rh/rd/rq spot checks.-test %/tests/lib/saloon-cplx ~— green (ok=%.y).-test %/tests/lib/saloon-eig-tol ~— green (ok=%.y).-test %/tests/lib/saloon-eigh-cc ~— green (ok=%.y).🤖 Generated with Claude Code