Skip to content

docs(disk-space): state the uniform arm's catch rate as measured - #574

Merged
rominf merged 1 commit into
mainfrom
docs/byte-generator-catch-rate
Oct 8, 2026
Merged

rominf merged 1 commit into
mainfrom
docs/byte-generator-catch-rate

Conversation

@rominf

@rominf rominf commented Oct 6, 2026

Copy link
Copy Markdown
Collaborator

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 is TiB, where format_bytes has 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.

  • If this PR fixes a bug, searched tests/e2e-cucumber/expectations.toml for the fixed ticket ID and removed/narrowed any now-stale xfail rows. (n/a, comment only)
  • If this PR adds a new subcommand or subsystem, its domain implementation lives in its own file per docs/architecture.md. (n/a)
  • Every new or changed user-facing message was read against the code path that runs after it. (n/a, no message changes)

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>
@rominf
rominf requested a review from a team as a code owner October 6, 2026 13:48
@rominf
rominf requested a review from siloteemu October 6, 2026 13:48

@jussielo-amd jussielo-amd left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approve

Blocking

None.

Non-blocking

None.

Notes

  • Doc-comment-only change (1 file, +4/-2) to the byte_count_strategy() test doc comment in crates/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 draws exponent uniformly over 0..=4 (B/KiB/MiB/GiB/TiB), and format_bytes's promotion loop (disk_space.rs:285-303) is gated by unit + 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.

@rominf
rominf added this pull request to the merge queue Oct 8, 2026
Merged via the queue into main with commit 2499351 Oct 8, 2026
30 of 31 checks passed
@rominf
rominf deleted the docs/byte-generator-catch-rate branch October 8, 2026 08:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants