docs(user): match the exp doc-block tolerances to the project's transcendental convention - #2552
docs(user): match the exp doc-block tolerances to the project's transcendental convention#2552georgebisbas wants to merge 1 commit into
Conversation
…cendental convention Five runnable doc blocks compare raw `exp` output against torch at `rtol=1e-4, atol=1e-4`, which is tighter than the device's `exp` is accurate. The assertions therefore sit on the edge and fail intermittently. Measured on `01-task-granularity.md:161`, whose inputs are seeded: expected values span 0.0032 to 253.5, so at the largest elements `rtol=1e-4` permits only ~0.017 absolute. The device lands 0.0259 off — a *relative* error of 1.49e-4, only 1.5x over budget — tripping 38-39 of 16384 elements (0.24%, the heavy tail). Three CI runs of hw-native-sys#2332, which does not touch these files, went fail / pass / fail, twice with a bit-identical worst-case error. `3e-3` is not an arbitrary loosening; it is the figure this repo already uses for the same class of value: - bounded / normalized outputs (softmax, layernorm, rms_norm) -> rtol=atol=1e-5 (tests/st/examples/02_intermediate/test_softmax.py etc.) - unbounded transcendental outputs (gelu / swiglu, i.e. exp-family) -> rtol=atol=3e-3 (tests/st/examples/02_intermediate/test_ffn_activations.py) Tight tolerances belong on outputs that are O(1) by construction. These blocks were applying one to unbounded `exp` output, 30x tighter than the project's own figure for transcendentals. At 3e-3 the observed 1.49e-4 passes with ~20x margin. Scope is only the assertions whose compared value is an `exp` result — 01-task-granularity :182/:186, 02-runtime-overhead :223, 04-incore :28, plus the zh mirrors. Every `c vs A + B` assertion is left at 1e-4, as is 02-runtime-overhead:169, whose `expected` is `A * 2.0` / `A * 2.0 + 1.0`. 02-runtime-overhead:223 and 04-incore:28 carried the same latent fragility and had simply not tripped yet. This explains why the assertions can fail; it does not explain why hw-native-sys#2332 tripped them in 2 of 4 runs while other recent runs, main included, did not. That asymmetry — and the bit-identical wrong value across two failures — points at build-level nondeterminism and is worth a separate look.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (6)
Included review availability: Your plan provides up to 2 included reviews per hour; 1 remains after this review. 📝 WalkthroughWalkthroughThe pull request relaxes ChangesPerformance documentation doctests
Estimated code review effort: 1 (Trivial) | ~3 minutes Merge Risk: ⚪ Minimal · up to This localized documentation change aligns exponential-output examples with the project’s established tolerance convention and has no actionable merge-blocking risk remaining after normal checks. Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Full details: Description checkExplanation The description directly explains the intermittent failures, the tolerance change, the affected English and Chinese documentation blocks, the unchanged arithmetic assertions, and the verification performed. Full details: Docstring CoverageExplanation No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 0 files. (6 skipped: 6 unsupported.) Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6fa5d78821
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| scratch, out = torch.zeros(LARGE, COLS), torch.zeros(LARGE, COLS) | ||
| two_tasks_via_gm(A[:LARGE], B[:LARGE], scratch, out, config=CFG) | ||
| torch.testing.assert_close(out, expected, rtol=1e-4, atol=1e-4) | ||
| torch.testing.assert_close(out, expected, rtol=3e-3, atol=3e-3) |
There was a problem hiding this comment.
When this page runs on x86_64 after the distributed and teaching simulator workloads, this assertion is the repository's existing reproducer for simulator-state contamination rather than ordinary exp approximation: .github/workflows/ci.yml:849-864 records identical seeded inputs and binaries producing different values, while lines 866-869 state that isolated arm64 runs match Torch exactly and that the architecture pin intentionally preserves the correctness signal. Raising the tolerance enough to accept the observed bad output will silently mask that contamination when x86_64 is re-enabled, so keep this check capable of detecting the documented mismatch or separate the approximation-tolerance test from the contamination reproducer.
Useful? React with 👍 / 👎.
|
Closing this — the premise is wrong, and @chatgpt-codex-connector's P2 caught it. That assertion is not merely tight: it is a live reproducer for a documented simulator-state contamination bug, and
Loosening the tolerance to For the record, my measurements independently reproduced the documented signature — bit-identical worst-case error with a varying mismatch count, The real cause of the recent failures is narrower than the tolerance: One observation worth keeping: the |
|
To use Codex here, create a Codex account and connect to github. |
1 participant
Problem
Five runnable doc blocks compare raw
expoutput against torch atrtol=1e-4, atol=1e-4. That is tighter than the device'sexpis accurate, so the assertions sit on the edge and fail intermittently.Measured on the failing one (
01-task-granularity.md:161, seeded inputs so this is reproducible): expected values span0.0032 … 253.5. At the largest elementsrtol=1e-4permits only ~0.017 absolute. The device lands 0.0259 off — a relative error of1.49e-4, i.e. only 1.5× over budget, tripping 38–39 of 16384 elements (0.24%, the heavy tail).Because the margin is that thin, the same block passes some CI runs and fails others. Observed on three runs of #2332 (which does not touch these files): fail, pass, fail — twice with a bit-identical worst-case error at the identical index.
Why
3e-3This repo already has a convention for exactly this distinction:
tests/st/examples/02_intermediate/test_softmax.py,test_layer_norm.py,test_rms_norm.pyrtol=atol=1e-5tests/st/examples/02_intermediate/test_ffn_activations.pyrtol=atol=3e-3Tight tolerances belong on outputs that are
O(1)by construction; transcendental results that can grow get3e-3. The doc blocks were using1e-4on unboundedexpoutput — 30× tighter than the project's own figure for this class. At3e-3the observed1.49e-4relative error passes with ~20× margin.This is an alignment to an existing convention, not a number tuned until CI went green.
Scope
Only the assertions whose compared value is an
expresult:01-task-granularity.md:182, :186 —expected = torch.exp(A[:LARGE] + B[:LARGE])02-runtime-overhead.md:223 —torch.exp(A[:TILE_ROWS] + B[:TILE_ROWS])04-incore.md:28 —torch.exp(A)plus the zh mirrors. Left untouched at
1e-4: everyc vs A + Bassertion, and02-runtime-overhead.md:169, whoseexpectedisA * 2.0/A * 2.0 + 1.0— plain arithmetic, correctly tight.Note
02-runtime-overhead.md:223and04-incore.md:28had the same latent fragility and simply had not tripped yet; fixing only the block that failed would have left them as landmines.Verification
pre-commit(incl. en/zh parity) andmkdocs build --strictclean.tests/docs/run_doc_examples.py --check-parity -p a2a3sim.What this does not claim
It explains why the assertion can fail. It does not explain why #2332 tripped it in 2 of 4 runs while ~19 other recent runs (main included) did not. A marginal assertion is exactly the kind that fails non-uniformly, but that asymmetry is unexplained and worth a separate look — as is the fact that two failing runs produced a bit-identical wrong value while a third run of the same commit produced a correct one, which hints at build-level nondeterminism rather than anything in the test.