Skip to content

fix(rocSHMEM): report bandwidth as GiB/s in reports, #AIROCSHMEM-561 - #14975

Merged
amirsojoodi merged 1 commit into
developfrom
users/asojoodi/rocshmem-tests-gibs
Nov 5, 2026
Merged

amirsojoodi merged 1 commit into
developfrom
users/asojoodi/rocshmem-tests-gibs

Conversation

@amirsojoodi

Copy link
Copy Markdown
Contributor

Motivation

The functional test computes bandwidth as bytes / time_s / pow(2, 30) dividing by 2^30, but labels and reports it as GB/s. That value is GiB/s, not GB/s.

  • tester.cpp: relabel the printed header GB/s -> GiB/s and rename the variable. The computation is unchanged; only the unit label now matches what is actually reported.
  • perf_compare.py: rename the matching DataFrame column and fix the docstring. The parser reads fixed columns by position, so this is an internal-consistency rename with no behavioral change.

Issue Tracking

JIRA ID: AIROCSHMEM-561

@amirsojoodi
amirsojoodi requested a review from a team as a code owner November 3, 2026 21:16
Copilot AI balanced review requested due to automatic review settings November 3, 2026 21:16
@therock-pr-bot

therock-pr-bot Bot commented Nov 3, 2026

Copy link
Copy Markdown

✅ All Policy Checks Passed

Check Status Details
📝 PR Description ✅ Pass —
⛔ Forbidden Files ✅ Pass —
🧪 Unit Test ⚠️ Warning Error: Source/code files changed without an accompanying unit test.
Expected: add at least one test file named like test_<name>.py / test_<name>.cpp (or <name>_test.*).
Current: code file(s) changed: projects/rocshmem/scripts/functional_tests/perf_compare.py, projects/rocshmem/tests/functional_tests/tester.cpp; no test file found
🚫 Draft PR 🔜 To Be Enabled —
🚩 Feature Flag 🔜 To Be Enabled —
📊 Code Coverage 🔜 To Be Enabled —

🎉 All policy checks passed!

📖 Need help? See the Policy FAQ for details on every check and how to fix failures.

🙋 Wish to Override Policy?

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟢 Approval recommended

There are no active review issues for the bandwidth-unit labeling and naming updates.

No open issues 🎉

What changed in this PR

Corrects rocSHMEM functional-test bandwidth units to GiB/s, matching the existing division by 2³⁰ without changing calculations or positional log parsing.

Changes:

  • Updates the printed bandwidth header and C++ variable name.
  • Renames the Python bandwidth column consistently and corrects the peak-bandwidth docstring.
File Description
projects/​rocshmem/​tests/​functional_tests/​tester.cpp Labels bandwidth as GiB/s and renames the corresponding variable.
projects/​rocshmem/​scripts/​functional_tests/​perf_compare.py Uses bandwidth_gibs throughout parsing and analysis; updates the unit documentation.

Review effort: Balanced

@math-ci

math-ci Bot commented Nov 3, 2026

Copy link
Copy Markdown

Resource usage report (4gfx1201)

Dashboard artifact

@math-ci

math-ci Bot commented Nov 4, 2026

Copy link
Copy Markdown

Resource usage report (8gfx942)

Dashboard artifact

@math-ci

math-ci Bot commented Nov 5, 2026

Copy link
Copy Markdown

Resource usage report (4gfx1201)

Dashboard artifact

@math-ci

math-ci Bot commented Nov 5, 2026

Copy link
Copy Markdown

Resource usage report (8gfx942)

Dashboard artifact

@amirsojoodi
amirsojoodi merged commit 96ed94b into develop Nov 5, 2026
95 of 101 checks passed
@amirsojoodi
amirsojoodi deleted the users/asojoodi/rocshmem-tests-gibs branch November 5, 2026 17:44
@systems-assistant

Copy link
Copy Markdown
Contributor

TheRock Submodule Bump Activity

Newest first

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants