Repository navigation
docs(disk-space): state the uniform arm's catch rate as measured - #574
Merged
Merged
Conversation
The top of the arm's exponent range is TiB, where the scaling loop has nothing left to promote to, so a too-early promoter renders exactly as the real function there. The arm catches it on two fifths of its draws, not half. Signed-off-by: Roman Inflianskas <Roman.Inflianskas@amd.com>
jussielo-amd
approved these changes
Oct 7, 2026
jussielo-amd
left a comment
Collaborator
There was a problem hiding this comment.
Approve
Blocking
None.
Non-blocking
None.
Notes
- Doc-comment-only change (1 file, +4/-2) to the
byte_count_strategy()test doc comment incrates/rocm-core/src/disk_space.rs:727-734. No executable code, tests, or behavior changed. - Verified the arithmetic against the actual implementation:
byte_count_strategy()'s middle arm drawsexponentuniformly over0..=4(B/KiB/MiB/GiB/TiB), andformat_bytes's promotion loop (disk_space.rs:285-303) is gated byunit + 1 < UNITS.len(), so the TiB range (exponent 4) can never promote - confirming "nothing left to promote to" for the largest range and the catch-rate math: (4 x ~50% + 1 x 0%) / 5 = 40% = "two fifths." - Cross-checked the PR's claimed measurement ("measured the arm at 40% over 200k draws, with 0 catches at exponent 4") against PR #503's review thread: volen-silo's inline comment there reports "caught on 40.07% of them... over 200,000 draws," and rominf's reply confirms the correction is exactly what's landed here. The new wording accurately reflects that verified finding.
- code-review (medium effort) returned zero findings.
- Leak scan on the real diff (merge-base compared) is clean; PR body contains no internal references.
- All CI checks green; no existing reviews/comments on the PR to reconcile.
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.
Follow-up to #503, from its approving review.
The byte generator's doc comment said a loop that promotes too early is caught on "about half" of the uniform arm's draws. The arm's exponent runs
0..=4, and the top range isTiB, whereformat_byteshas nothing left to promote to. There a half-unit promoter renders exactly what the real function does, so the property cannot tell them apart. The reviewer measured the arm at 40% over 200k draws, with 0 catches at exponent 4. The comment now says "about half in every range but the largest — two fifths of this arm's draws". The "within a handful of draws across the whole strategy" half was already right (20% per draw overall).Comment-only; no code or test changes.
tests/e2e-cucumber/expectations.tomlfor the fixed ticket ID and removed/narrowed any now-stale xfail rows. (n/a, comment only)docs/architecture.md. (n/a)