Repository navigation
fix(primbench): m_last_bytes_per_second was always zero, ruining autotuning - #11760
Merged
Merged
Conversation
✅ All Checks Passed — Ready for Review
📖 Need help? See the Policy FAQ for details on every check and how to fix failures. |
|
🎉 All checks passed! This PR is ready for review. |
MyNameIsTrez
marked this pull request as draft
September 8, 2026 11:09
NB4444
force-pushed
the
fix-last-bytes-per-second-being-zero
branch
from
September 8, 2026 13:31
0222627 to
ff8e017
Compare
NguyenNhuDi
approved these changes
Sep 8, 2026
1 task
NB4444
force-pushed
the
fix-last-bytes-per-second-being-zero
branch
2 times, most recently
from
September 9, 2026 09:40
101a044 to
9c3c303
Compare
NB4444
marked this pull request as ready for review
September 9, 2026 09:41
…totuning I followed the instructions in projects/rocprim/tuning/README.md to confirm that the generated default config now matches the fastest specialization. The reason this problem was hidden is because the autotuning system always picked the existing default config, which was already the fastest specialization. By passing `--exclude-default-config` to `run_tuning.py` the autotuning system before this fix commit would output terribly slow configs.
In rocPRIM there are several benchmarks which call `state.run()` multiple times during tuning, like `projects/rocprim/benchmark/benchmark_device_segmented_reduce.hpp` calling it for 5 different segment sizes. This caused the bytes/sec that was returned to Kernel Tuner to be inaccurate, and for the same specialization to be printed 5x in the terminal, which was very confusing.
NB4444
force-pushed
the
fix-last-bytes-per-second-being-zero
branch
from
September 9, 2026 10:45
c3d366f to
5955278
Compare
NguyenNhuDi
approved these changes
Sep 9, 2026
NguyenNhuDi
left a comment
Contributor
There was a problem hiding this comment.
This still LGTM. However due to the coming 10.1 snap we should refrain from merging until after.
assistant-librarian Bot
pushed a commit
to ROCm/rocPRIM
that referenced
this pull request
Sep 14, 2026
fix(primbench): `m_last_bytes_per_second` was always zero, ruining autotuning (#11760) ## Motivation JIRA ID : EXSWSTRHPC-322 `m_last_bytes_per_second` was always zero, ruining autotuning. Additionally, some rocPRIM benchmarks (such as `projects/rocprim/benchmark/benchmark_device_segmented_reduce.hpp`) mistakenly called `state.run()` multiple times during tuning to test different segment sizes. This caused the bytes/sec returned to Kernel Tuner to be inaccurate and printed the same specialization multiple times to the terminal, which was very confusing. ## Technical Details The `m_last_bytes_per_second` variable is read by [rocprim/tuning/tuner/templates/common/base.cpp.jinja2](https://github.com/ROCm/rocm-libraries/blob/bee97df517907c771de17189cb867d3c401285ae/projects/rocprim/tuning/tuner/templates/common/base.cpp.jinja2#L56-L59): ```c++ executor.run(); // Return the time as milliseconds per GiB. return (1000 * primbench::GiB) / executor.get_last_bytes_per_second(); ``` This problem was hidden because the autotuning system always picked the existing default config. By passing `--exclude-default-config` to `run_tuning.py` I told it not to use rocPRIM's default config. As expected, this caused a random, slow config to be output, as Kernel Tuner thought every config had an identical speed (`m_last_bytes_per_second`). To prevent downstream autotuning issues, `primbench.hpp` was updated to explicitly forbid calling `state.run()` more than once per state object, matching the existing `set_items` behavior. ## Test Plan Here are the commands I ran while following the instructions in `rocprim/tuning/README.md`: ```sh # Change directory cd projects/rocprim/tuning # Activate virtual environment python -m venv .venv source .venv/bin/activate # Install dependencies pip3 install -i https://test.pypi.org/simple hip-python pip3 install jinja2 kernel-tuner numpy # Run tuning, using Ctrl+\ to stop the process once it finished outputting the first JSON file rm -f output/device_merge_gfx90a_int128_t_cache.json && \ ./run_tuning.py --algo-regex device_merge --seed 42 # Generate configuration ./confgen/generate.py \ --input output/device_merge_gfx90a_int128_t_cache.json \ --output foo --map-gfx gfx90a mi210 ``` To verify the new safeguard, I also intentionally modified `shared/primbench/examples/hip/copy_benchmark.cpp` to call `state.run()` twice in succession. ## Test Result I looked at the terminal output, JSON output, and generated config to confirm the generated config now always matches the fastest specialization. I also confirmed that my test modification in `copy_benchmark.cpp` successfully threw the `"Error: Can't call run() twice"` error and cleanly aborted execution as intended. I also manually checked some specializations of the changed benchmark, to see if the results are the same, and they seemed to be. There is some run-to-run variance, especially with the segmented ones, but that was the same for both versions of the benchmarks. ## Submission Checklist - [x] Look over the contributing guidelines at https://github.com/ROCm/ROCm/blob/develop/CONTRIBUTING.md#pull-requests.
Contributor
TheRock Submodule Bump ActivityNewest first
|
stanleytsang-amd
pushed a commit
that referenced
this pull request
Sep 18, 2026
…totuning (#11760) ## Motivation JIRA ID : EXSWSTRHPC-322 `m_last_bytes_per_second` was always zero, ruining autotuning. Additionally, some rocPRIM benchmarks (such as `projects/rocprim/benchmark/benchmark_device_segmented_reduce.hpp`) mistakenly called `state.run()` multiple times during tuning to test different segment sizes. This caused the bytes/sec returned to Kernel Tuner to be inaccurate and printed the same specialization multiple times to the terminal, which was very confusing. ## Technical Details The `m_last_bytes_per_second` variable is read by [rocprim/tuning/tuner/templates/common/base.cpp.jinja2](https://github.com/ROCm/rocm-libraries/blob/bee97df517907c771de17189cb867d3c401285ae/projects/rocprim/tuning/tuner/templates/common/base.cpp.jinja2#L56-L59): ```c++ executor.run(); // Return the time as milliseconds per GiB. return (1000 * primbench::GiB) / executor.get_last_bytes_per_second(); ``` This problem was hidden because the autotuning system always picked the existing default config. By passing `--exclude-default-config` to `run_tuning.py` I told it not to use rocPRIM's default config. As expected, this caused a random, slow config to be output, as Kernel Tuner thought every config had an identical speed (`m_last_bytes_per_second`). To prevent downstream autotuning issues, `primbench.hpp` was updated to explicitly forbid calling `state.run()` more than once per state object, matching the existing `set_items` behavior. ## Test Plan Here are the commands I ran while following the instructions in `rocprim/tuning/README.md`: ```sh # Change directory cd projects/rocprim/tuning # Activate virtual environment python -m venv .venv source .venv/bin/activate # Install dependencies pip3 install -i https://test.pypi.org/simple hip-python pip3 install jinja2 kernel-tuner numpy # Run tuning, using Ctrl+\ to stop the process once it finished outputting the first JSON file rm -f output/device_merge_gfx90a_int128_t_cache.json && \ ./run_tuning.py --algo-regex device_merge --seed 42 # Generate configuration ./confgen/generate.py \ --input output/device_merge_gfx90a_int128_t_cache.json \ --output foo --map-gfx gfx90a mi210 ``` To verify the new safeguard, I also intentionally modified `shared/primbench/examples/hip/copy_benchmark.cpp` to call `state.run()` twice in succession. ## Test Result I looked at the terminal output, JSON output, and generated config to confirm the generated config now always matches the fastest specialization. I also confirmed that my test modification in `copy_benchmark.cpp` successfully threw the `"Error: Can't call run() twice"` error and cleanly aborted execution as intended. I also manually checked some specializations of the changed benchmark, to see if the results are the same, and they seemed to be. There is some run-to-run variance, especially with the segmented ones, but that was the same for both versions of the benchmarks. ## Submission Checklist - [x] Look over the contributing guidelines at https://github.com/ROCm/ROCm/blob/develop/CONTRIBUTING.md#pull-requests. --------- Co-authored-by: Nick Breed <nick@streamhpc.com> Co-authored-by: Di Nguyen <dinguyennhu@gmail.com>
ammallya
pushed a commit
that referenced
this pull request
Sep 24, 2026
## Motivation Implement Kernel Tuner for `adjacent` algorithms. JIRA ID: AIPRIMS-236 & EXSWSTRHPC-322 ## Technical Details The rocPRIM config changes are due to running `migrate.py` (this updates the legacy config to be ready for Kernel Tuner) Do not merge this PR until ~~#10790~~, ~~#11109~~ ~~#11760~~ is merged. Recommended way to review this PR is to go to "Commit" and review the files per commit. ## Test Plan Local running of kernel tuner, making sure output matches legacy tuners and that there are no errors. ## Test Result No errors observed for `adjacent_find`, ## Submission Checklist - [ ] Look over the contributing guidelines at https://github.com/ROCm/ROCm/blob/develop/CONTRIBUTING.md#pull-requests.
illsilin
pushed a commit
to igorluppi/rocm-libraries
that referenced
this pull request
Oct 1, 2026
…totuning (ROCm#11760) ## Motivation JIRA ID : EXSWSTRHPC-322 `m_last_bytes_per_second` was always zero, ruining autotuning. Additionally, some rocPRIM benchmarks (such as `projects/rocprim/benchmark/benchmark_device_segmented_reduce.hpp`) mistakenly called `state.run()` multiple times during tuning to test different segment sizes. This caused the bytes/sec returned to Kernel Tuner to be inaccurate and printed the same specialization multiple times to the terminal, which was very confusing. ## Technical Details The `m_last_bytes_per_second` variable is read by [rocprim/tuning/tuner/templates/common/base.cpp.jinja2](https://github.com/ROCm/rocm-libraries/blob/bee97df517907c771de17189cb867d3c401285ae/projects/rocprim/tuning/tuner/templates/common/base.cpp.jinja2#L56-L59): ```c++ executor.run(); // Return the time as milliseconds per GiB. return (1000 * primbench::GiB) / executor.get_last_bytes_per_second(); ``` This problem was hidden because the autotuning system always picked the existing default config. By passing `--exclude-default-config` to `run_tuning.py` I told it not to use rocPRIM's default config. As expected, this caused a random, slow config to be output, as Kernel Tuner thought every config had an identical speed (`m_last_bytes_per_second`). To prevent downstream autotuning issues, `primbench.hpp` was updated to explicitly forbid calling `state.run()` more than once per state object, matching the existing `set_items` behavior. ## Test Plan Here are the commands I ran while following the instructions in `rocprim/tuning/README.md`: ```sh # Change directory cd projects/rocprim/tuning # Activate virtual environment python -m venv .venv source .venv/bin/activate # Install dependencies pip3 install -i https://test.pypi.org/simple hip-python pip3 install jinja2 kernel-tuner numpy # Run tuning, using Ctrl+\ to stop the process once it finished outputting the first JSON file rm -f output/device_merge_gfx90a_int128_t_cache.json && \ ./run_tuning.py --algo-regex device_merge --seed 42 # Generate configuration ./confgen/generate.py \ --input output/device_merge_gfx90a_int128_t_cache.json \ --output foo --map-gfx gfx90a mi210 ``` To verify the new safeguard, I also intentionally modified `shared/primbench/examples/hip/copy_benchmark.cpp` to call `state.run()` twice in succession. ## Test Result I looked at the terminal output, JSON output, and generated config to confirm the generated config now always matches the fastest specialization. I also confirmed that my test modification in `copy_benchmark.cpp` successfully threw the `"Error: Can't call run() twice"` error and cleanly aborted execution as intended. I also manually checked some specializations of the changed benchmark, to see if the results are the same, and they seemed to be. There is some run-to-run variance, especially with the segmented ones, but that was the same for both versions of the benchmarks. ## Submission Checklist - [x] Look over the contributing guidelines at https://github.com/ROCm/ROCm/blob/develop/CONTRIBUTING.md#pull-requests. --------- Co-authored-by: Nick Breed <nick@streamhpc.com> Co-authored-by: Di Nguyen <dinguyennhu@gmail.com>
Naraenda
pushed a commit
that referenced
this pull request
Oct 2, 2026
Implement Kernel Tuner for `adjacent` algorithms. JIRA ID: AIPRIMS-236 & EXSWSTRHPC-322 The rocPRIM config changes are due to running `migrate.py` (this updates the legacy config to be ready for Kernel Tuner) Do not merge this PR until ~~#10790~~, ~~#11109~~ ~~#11760~~ is merged. Recommended way to review this PR is to go to "Commit" and review the files per commit. Local running of kernel tuner, making sure output matches legacy tuners and that there are no errors. No errors observed for `adjacent_find`, - [ ] Look over the contributing guidelines at https://github.com/ROCm/ROCm/blob/develop/CONTRIBUTING.md#pull-requests.
stanleytsang-amd
pushed a commit
that referenced
this pull request
Oct 2, 2026
Implement Kernel Tuner for `adjacent` algorithms. JIRA ID: AIPRIMS-236 & EXSWSTRHPC-322 The rocPRIM config changes are due to running `migrate.py` (this updates the legacy config to be ready for Kernel Tuner) Do not merge this PR until ~~#10790~~, ~~#11109~~ ~~#11760~~ is merged. Recommended way to review this PR is to go to "Commit" and review the files per commit. Local running of kernel tuner, making sure output matches legacy tuners and that there are no errors. No errors observed for `adjacent_find`, - [ ] Look over the contributing guidelines at https://github.com/ROCm/ROCm/blob/develop/CONTRIBUTING.md#pull-requests.
Naraenda
pushed a commit
that referenced
this pull request
Oct 5, 2026
Implement Kernel Tuner for `adjacent` algorithms. JIRA ID: AIPRIMS-236 & EXSWSTRHPC-322 The rocPRIM config changes are due to running `migrate.py` (this updates the legacy config to be ready for Kernel Tuner) Do not merge this PR until ~~#10790~~, ~~#11109~~ ~~#11760~~ is merged. Recommended way to review this PR is to go to "Commit" and review the files per commit. Local running of kernel tuner, making sure output matches legacy tuners and that there are no errors. No errors observed for `adjacent_find`, - [ ] Look over the contributing guidelines at https://github.com/ROCm/ROCm/blob/develop/CONTRIBUTING.md#pull-requests.
Naraenda
pushed a commit
that referenced
this pull request
Oct 7, 2026
Implement Kernel Tuner for `adjacent` algorithms. JIRA ID: AIPRIMS-236 & EXSWSTRHPC-322 The rocPRIM config changes are due to running `migrate.py` (this updates the legacy config to be ready for Kernel Tuner) Do not merge this PR until ~~#10790~~, ~~#11109~~ ~~#11760~~ is merged. Recommended way to review this PR is to go to "Commit" and review the files per commit. Local running of kernel tuner, making sure output matches legacy tuners and that there are no errors. No errors observed for `adjacent_find`, - [ ] Look over the contributing guidelines at https://github.com/ROCm/ROCm/blob/develop/CONTRIBUTING.md#pull-requests.
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.
Motivation
JIRA ID : EXSWSTRHPC-322
m_last_bytes_per_secondwas always zero, ruining autotuning.Additionally, some rocPRIM benchmarks (such as
projects/rocprim/benchmark/benchmark_device_segmented_reduce.hpp) mistakenly calledstate.run()multiple times during tuning to test different segment sizes. This caused the bytes/sec returned to Kernel Tuner to be inaccurate and printed the same specialization multiple times to the terminal, which was very confusing.Technical Details
The
m_last_bytes_per_secondvariable is read by rocprim/tuning/tuner/templates/common/base.cpp.jinja2:This problem was hidden because the autotuning system always picked the existing default config. By passing
--exclude-default-configtorun_tuning.pyI told it not to use rocPRIM's default config. As expected, this caused a random, slow config to be output, as Kernel Tuner thought every config had an identical speed (m_last_bytes_per_second).To prevent downstream autotuning issues,
primbench.hppwas updated to explicitly forbid callingstate.run()more than once per state object, matching the existingset_itemsbehavior.Test Plan
Here are the commands I ran while following the instructions in
rocprim/tuning/README.md:To verify the new safeguard, I also intentionally modified
shared/primbench/examples/hip/copy_benchmark.cppto callstate.run()twice in succession.Test Result
I looked at the terminal output, JSON output, and generated config to confirm the generated config now always matches the fastest specialization.
I also confirmed that my test modification in
copy_benchmark.cppsuccessfully threw the"Error: Can't call run() twice"error and cleanly aborted execution as intended.I also manually checked some specializations of the changed benchmark, to see if the results are the same, and they seemed to be. There is some run-to-run variance, especially with the segmented ones, but that was the same for both versions of the benchmarks.
Submission Checklist