Skip to content

feat(rocthrust): use primbench#8499

Open
MyNameIsTrez wants to merge 12 commits into
developfrom
users/mynameistrez/use-primbench-in-rocthrust
Open

feat(rocthrust): use primbench#8499
MyNameIsTrez wants to merge 12 commits into
developfrom
users/mynameistrez/use-primbench-in-rocthrust

Conversation

@MyNameIsTrez

@MyNameIsTrez MyNameIsTrez commented Jun 16, 2026

Copy link
Copy Markdown
Contributor

Motivation

Caution

This PR must not be merged before the PR feat(rocthrust): allow grafting thrust onto benchmarks on which it is based has been merged.

rocThrust's benchmarks have been updated to use primbench. This makes the benchmark results more accurate than was possible with Google Benchmark.

The diff of for_each/basic.cu represents what changes have been made to all benchmarks.

JIRA ID : AIPRIMS-208

Technical Details

Note

If AMD's GPU cooling setup is too weak or too powerful then --min-gpu-temp (default 50°C) or --max-gpu-temp (default 60°C) will need to be passed to benchmarks by AMD's CI. I recommend holding off on passing these flags until a benchmark actually times out from not reaching the 50°C-60°C temperature range.

Test Plan

The below blocks of commands can be copied directly into a terminal, so you don't need to copy the lines individually.

See the Test Result header in this description for the zip that contains all of the scripts that these reproduction steps use.

VS Code dev containers

HIP Dockerfile + devcontainer.json

Dockerfile:

FROM rocm/rocm-terminal:latest

devcontainer.json:

{
    "build": {
        "dockerfile": "Dockerfile"
    },
    "name": "hip-minimal",
    "runArgs": [
        "--device=/dev/kfd",
        "--device=/dev/dri"
    ]
}
CUDA Dockerfile + devcontainer.json

Dockerfile:

FROM nvidia/cuda:12.9.1-devel-ubuntu24.04

RUN apt update && apt install -y git cmake ninja-build wget

RUN wget -qO- https://repo.radeon.com/rocm/rocm.gpg.key | gpg --dearmor > /etc/apt/keyrings/rocm.gpg

RUN echo "deb [arch=amd64 signed-by=/etc/apt/keyrings/rocm.gpg] https://repo.radeon.com/rocm/apt/7.2 noble main" > /etc/apt/sources.list.d/rocm.list

RUN tee /etc/apt/preferences.d/rocm-pin-600 <<'EOF'
Package: *
Pin: origin repo.radeon.com
Pin-Priority: 600
EOF

RUN apt update && apt install -y hip-base

ENV HIP_PLATFORM=nvidia

devcontainer.json:

{
    "build": {
        "dockerfile": "Dockerfile"
    },
    "name": "cuda-minimal",
    "privileged": true,
    "runArgs": [
        "--gpus=all"
    ]
}

Reproduction steps

Installing dependencies
sudo apt update && \
sudo apt install -y cmake ninja-build
Cloning
git clone --no-checkout --filter=blob:none https://github.com/ROCm/rocm-libraries.git && \
cd rocm-libraries && \
git sparse-checkout init --cone && \
git sparse-checkout set projects/rocthrust shared/primbench
Building primbench benchmarks

Setup

git checkout users/mynameistrez/use-primbench-in-rocthrust && \
cd projects/rocthrust && \
mkdir build_primbench && \
cd build_primbench

HIP

export gfx=$(rocm_agent_enumerator | head -n1) && \
CXX=hipcc cmake -GNinja -DBUILD_BENCHMARK=ON -DGPU_TARGETS="$gfx" .. && \
ninja && \
cd ..

CUDA

TODO: Add
Running primbench benchmarks
python3 .gitlab/run_benchmarks.py \
  --benchmark-executables-dir=build_primbench/benchmark \
  --gpu-architecture=$gfx \
  --json-out-dir=results_primbench \
  --csv-out-dir=results_primbench
Copying primbench benchmark results from a server
scp \
  -r \
  myname@myserver:~/rocm-libraries/projects/rocthrust/results_primbench \
  ~/Downloads
Building develop benchmarks

Setup

# TODO: Update this git commit hash with whatever commit this PR eventually gets rebased onto:
git checkout 53cfd867 && \
cd projects/rocthrust && \
mkdir build_develop && \
cd build_develop

HIP

export gfx=$(rocm_agent_enumerator | head -n1) && \
CXX=hipcc cmake -GNinja -DBUILD_BENCHMARK=ON -DGPU_TARGETS="$gfx" .. && \
ninja && \
cd ..

CUDA

TODO: Add
Running develop benchmarks
mkdir results_develop && \
python3 .gitlab/run_benchmarks.py \
  --benchmark_dir=build_develop/benchmark \
  --benchmark_gpu_architecture="$gfx" \
  --benchmark_output_dir=results_develop
Copying develop benchmark results from a server
scp \
  -r \
  myname@myserver:~/rocm-libraries/projects/rocthrust/results_develop \
  ~/Downloads

Test Result

No meaningful performance regressions and noise timeouts were observed.

I ran all 40 rocThrust benchmarks on a GFX1201 GPU.

Noisy Benchmarks

1 benchmark still has more than 1% noise: set_operations_difference. settings.noise_tolerance_percent = 2 was set to prevent noise timeout (actual noise is around 1.1%).

Paul Kliemann is working on a primbench PR that will significantly lower the noise of benchmark specializations, by calculating entropy instead. hipCUB will automatically benefit from it.

Submission Checklist

@codecov-commenter

codecov-commenter commented Jun 16, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

❌ Your project status has failed because the head coverage (76.84%) is below the target coverage (80.00%). You can increase the head coverage or adjust the target coverage.

Additional details and impacted files
@@             Coverage Diff             @@
##           develop    #8499      +/-   ##
===========================================
+ Coverage    64.60%   64.61%   +0.01%     
===========================================
  Files         2687     2686       -1     
  Lines       423273   423184      -89     
  Branches     62933    62925       -8     
===========================================
- Hits        273439   273414      -25     
+ Misses      129163   129097      -66     
- Partials     20671    20673       +2     
Flag Coverage Δ *Carryforward flag
TensileLite 34.72% <ø> (+0.02%) ⬆️ Carriedforward from 6d3de69
hipBLAS 90.81% <ø> (ø) Carriedforward from 6d3de69
hipBLASLt 34.75% <ø> (ø) Carriedforward from 6d3de69
hipCUB 82.68% <ø> (ø) Carriedforward from 6d3de69
hipDNN 86.03% <ø> (-0.01%) ⬇️ Carriedforward from 6d3de69
hipFFT 50.88% <ø> (ø) Carriedforward from 6d3de69
hipRAND 76.12% <ø> (ø) Carriedforward from 6d3de69
hipSOLVER 69.18% <ø> (ø) Carriedforward from 6d3de69
hipSPARSE 86.10% <ø> (ø) Carriedforward from 6d3de69
rocBLAS 47.92% <ø> (ø) Carriedforward from 6d3de69
rocFFT 46.82% <ø> (ø) Carriedforward from 6d3de69
rocRAND 57.03% <ø> (ø)
rocSOLVER 76.84% <ø> (ø) Carriedforward from 6d3de69
rocSPARSE 72.37% <ø> (-<0.01%) ⬇️ Carriedforward from 6d3de69
rocThrust 91.36% <ø> (ø) Carriedforward from 6d3de69

*This pull request uses carry forward flags. Click here to find out more.
see 9 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@MyNameIsTrez
MyNameIsTrez force-pushed the users/mynameistrez/use-primbench-in-rocthrust branch 2 times, most recently from e48fde6 to 7babd0a Compare June 17, 2026 12:32
Comment thread projects/rocthrust/benchmark/CMakeLists.txt Outdated
@umfranzw

umfranzw commented Jun 24, 2026

Copy link
Copy Markdown
Contributor

I've got a TheRock PR up here to make amd_smi available for rocThrust: ROCm/TheRock#6100.
It'll need to be merged before CI will pass here.

Comment thread projects/rocthrust/benchmark/bench/transform/basic.cu Outdated
Comment thread projects/rocthrust/benchmark/bench/transform/basic.cu Outdated
Comment thread projects/rocthrust/benchmark/bench/transform/basic.cu Outdated
Comment thread projects/rocthrust/benchmark/bench_utils.hpp Outdated
@NguyenNhuDi

Copy link
Copy Markdown
Contributor

@MyNameIsTrez, I applied the recommendation to turn CHECK_VALID into a function.

Comment thread projects/rocthrust/benchmark/bench/vectorized_search/basic.cu Outdated
Comment thread projects/rocthrust/benchmark/bench/vectorized_search/lower_bound.cu Outdated
Comment thread projects/rocthrust/benchmark/bench/vectorized_search/upper_bound.cu Outdated
@NguyenNhuDi

Copy link
Copy Markdown
Contributor

@MyNameIsTrez I have removed all the state.write(0) calls

@therock-pr-bot

therock-pr-bot Bot commented Jul 3, 2026

Copy link
Copy Markdown

❌ PR Check — Action Required

Check Status Details
🌿 Branch Name ✅ Pass
📝 PR Title/Description ✅ Pass
Forbidden Files ✅ Pass
🧪 Unit Test ❌ Fail 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/rocthrust/.gitlab/run_benchmarks.py, projects/rocthrust/benchmark/bench/set_operations/base.hpp, projects/rocthrust/benchmark/bench/set_operations/by_key.hpp, projects/rocthrust/benchmark/bench_utils.hpp, projects/rocthrust/benchmark/generation_utils.hpp (+1 more); no test file found
🔎 pre-commit ✅ Pass
🚫 Draft PR 🔜 To Be Enabled
🚩 Feature Flag 🔜 To Be Enabled
📊 Code Coverage 🔜 To Be Enabled

⚠️ 1 policy check(s) failed. Please address the issues above before this PR can be Reviewed.

🚫 Please fix the failed policies

  • ❌ Unit Test

The Not ready to Review label was added to this PR. Once all policies pass, the label is removed automatically.

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

@therock-pr-bot

therock-pr-bot Bot commented Jul 3, 2026

Copy link
Copy Markdown

🚫 Please fix the failed policies before requesting reviews.

The following policy checks failed:

  • ❌ Unit Test

The Not ready to Review label has been added to this PR.
Once all policies pass, the label will be removed automatically.

@MyNameIsTrez

MyNameIsTrez commented Jul 3, 2026

Copy link
Copy Markdown
Contributor Author

In case anyone ever asks in the future btw @NguyenNhuDi, the two gifs in primbench its readme were generated using asciinema + agg.

Here are the details from my personal notes on how I produced those gifs, in case they ever need to be updated. Feel free to copy my notes to a more permanent spot, and to modify them:

Details

Context

asciinema is one of the most popular command-line tools to record your terminal with.

agg (asciinema gif generator) is a command-line tool for generating GIFs from terminal recordings.

Installation

You can add this to your Dockerfile to download asciinema:

RUN sudo apt update && sudo apt install asciinema -y

And you can add this to your Dockerfile to download agg:

RUN sudo wget -O /usr/local/bin/agg https://github.com/asciinema/agg/releases/download/v1.7.0/agg-x86_64-unknown-linux-gnu && \
    sudo chmod +x /usr/local/bin/agg

Usage

Run asciinema --help and agg --help.

My typical workflow is:

  1. Running asciinema record recording.cast.
  2. Hitting Ctrl+D when I am done recording, which will output recording.cast.
  3. Manually changing the width and height values from the first line of recording.cast.
  4. Manually removing lines from recording.cast that I don't want to show up in the gif.
  5. Modifying the below trim.py script its trim_start_secs variable.
    • If the first line in recording.cast starts at 4.56 seconds, then you can assign that value to trim_start_secs.
  6. Running python3 trim.py.
  7. Generating recording.gif using agg recording_trimmed.cast recording.gif --speed 1.0 --theme asciinema --last-frame-duration 0 --font-size 32 --renderer fontdue.
import json

infile = "recording.cast"
outfile = "recording_trimmed.cast"
trim_start_secs = 1.23

with open(infile) as f:
    lines = f.readlines()

header = json.loads(lines[0])
events = [json.loads(line) for line in lines[1:]]

# Filter and shift timestamps
events = [e for e in events if e[0] >= trim_start_secs]
events = [[round(e[0] - trim_start_secs, 6), e[1], e[2]] for e in events]

with open(outfile, "w") as f:
    json.dump(header, f, ensure_ascii=False)
    f.write("\n")
    for e in events:
        json.dump(e, f, ensure_ascii=False)
        f.write("\n")

@MyNameIsTrez

MyNameIsTrez commented Jul 9, 2026

Copy link
Copy Markdown
Contributor Author

@NguyenNhuDi I left some review comments on your commits, in case you hadn't seen them. GitHub apparently hides such review comments, where you have to click this chat icon to view them:

image

@NguyenNhuDi

Copy link
Copy Markdown
Contributor

@MyNameIsTrez thanks for letting me know. Super strange that it won't show unless you click on it

@MyNameIsTrez

MyNameIsTrez commented Jul 10, 2026

Copy link
Copy Markdown
Contributor Author

@NguyenNhuDi The chat icon is more visible on the commits page it turns out, so I recommend scrolling from top-to-bottom there to find the other comments I left:

image

@NguyenNhuDi
NguyenNhuDi force-pushed the users/mynameistrez/use-primbench-in-rocthrust branch from 22d2ea2 to 279dc80 Compare July 14, 2026 18:08
@NguyenNhuDi
NguyenNhuDi marked this pull request as ready for review July 14, 2026 18:10
@NguyenNhuDi
NguyenNhuDi requested review from a team as code owners July 14, 2026 18:10
@NguyenNhuDi

Copy link
Copy Markdown
Contributor

@MyNameIsTrez I have squashed the commits into chunks that makes sense. I believe that this PR is now ready to be reviewed.

@MyNameIsTrez

MyNameIsTrez commented Jul 15, 2026

Copy link
Copy Markdown
Contributor Author

@NguyenNhuDi Can you update the description to have this "Test Result" header from hipCUB's PR, which summarizes your results? (without any data)

I recommend pressing the Edit button on that hipCUB PR's description, to copy its Test Result header into your rocThrust PR's description, where you update it based on your rocThrust methodology and results.


image

@MyNameIsTrez

Copy link
Copy Markdown
Contributor Author

@NguyenNhuDi Can you confirm that the updated run_benchmarks.py successfully runs all benchmarks? (pass --dry to it to have it quickly verify all benchmarks work)

@MyNameIsTrez

Copy link
Copy Markdown
Contributor Author

@NguyenNhuDi Great work! Seeing this makes me happy:

image

Comment thread projects/rocthrust/benchmark/bench/transform/basic.cu Outdated
Comment thread projects/rocthrust/benchmark/bench/unique/by_key.cu Outdated
Comment thread projects/rocthrust/benchmark/generation_utils.hpp Outdated
@MyNameIsTrez

MyNameIsTrez commented Jul 15, 2026

Copy link
Copy Markdown
Contributor Author

@NguyenNhuDi Make sure to heed this caution from the top of the readme. Nara has finished that PR of hers just now, so if you approve and merge it then you can rebase your own rocThrust PR onto it immediately:

Caution

This PR must not be merged before the PR feat(rocthrust): allow grafting thrust onto benchmarks on which it is based has been merged.

@MyNameIsTrez

MyNameIsTrez commented Jul 15, 2026

Copy link
Copy Markdown
Contributor Author

I'm done reviewing all of the code now.

@NguyenNhuDi

Copy link
Copy Markdown
Contributor

@MyNameIsTrez I've generated both cold and hot benchmarks using the run_benchmark script in the gitlab folder. In fact thats what I've used this entire time to generate my benchmarks

@NguyenNhuDi

Copy link
Copy Markdown
Contributor

@MyNameIsTrez I've taken a look at your review! Thanks for that :)
I've made the suggested changes if you could take a look once more

@MyNameIsTrez

MyNameIsTrez commented Jul 16, 2026

Copy link
Copy Markdown
Contributor Author

@NguyenNhuDi It seems you didn't remove the extra spaces before the : from the commits yet. Could you do that too? Everything else looks good. Thanks.

@NguyenNhuDi
NguyenNhuDi force-pushed the users/mynameistrez/use-primbench-in-rocthrust branch from aca3631 to 28ba91a Compare July 17, 2026 15:45
@NguyenNhuDi

Copy link
Copy Markdown
Contributor

@MyNameIsTrez oops I did forget to change the commit names. I've done that now

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.

5 participants