Repository navigation
fix(rocrand): fix race condition in poisson distribution - #14419
Merged
stanleytsang-amd merged 3 commits intoOct 29, 2026
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. |
Contributor
|
Change looks fine with me, please update changelog and cmake packaging version to reflect this bugfix for ROCm 10.3 |
Contributor
Author
It is done in 07246ec. Thanks @stanleytsang-amd |
stanleytsang-amd
approved these changes
Oct 26, 2026
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #14419 +/- ##
===========================================
+ Coverage 76.89% 76.92% +0.04%
===========================================
Files 2886 2886
Lines 481037 480811 -226
Branches 72377 72377
===========================================
+ Hits 369847 369850 +3
+ Misses 86832 86601 -231
- Partials 24358 24360 +2
*This pull request uses carry forward flags. Click here to find out more.
🚀 New features to boost your workflow:
|
spolifroni-amd
approved these changes
Oct 28, 2026
spolifroni-amd
left a comment
spolifroni-amd
left a comment
Contributor
There was a problem hiding this comment.
Just need a period in the changelog entry.
Co-authored-by: spolifroni-amd <Sandra.Polifroni@amd.com>
stanleytsang-amd
deleted the
users/NguyenNhuDi/fix-rocrand-race-condition
branch
October 29, 2026 15:20
assistant-librarian Bot
pushed a commit
to ROCm/rocRAND
that referenced
this pull request
Oct 29, 2026
fix(rocrand): fix race condition in poisson distribution (#14419) ## Motivation QA reported a races condition in `poisson` distribution. This PR aims to fix this. JIRA ID: ROCM-32132 ## Technical Details Using `TSAN` QA reported that inside the `poisson` distribution when calling `std::lgamma` inside `calculate_poisson_probabilities` there was a race condition. This is happening because `std::lgamma` stores the sign of the output inside a global variable. To fix this we can use `lgamma_r` which allows us to store the sign locally without storing it in a global variable. Since `lgamma_r` is not part of the `stdlib` and is only implemented in `glibc`, it is not available for windows (`msvc`). This is the reason why we need to check if we are currently on windows or not. ## Test Plan Standard CI test suite. Also local testing on both linux and windows. Also re-run with `TSAN` to see if race condition is still reported. ## Test Result Local unit test ran on linux and windows shows no failures. `TSAN` run shows no race condition originated from rocrand. ## Submission Checklist - [ ] Look over the contributing guidelines at https://github.com/ROCm/TheRock/blob/main/GOVERNANCE.md#pull-requests.
Contributor
TheRock Submodule Bump ActivityNewest first
|
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
QA reported a races condition in
poissondistribution. This PR aims to fix this.JIRA ID: ROCM-32132
Technical Details
Using
TSANQA reported that inside thepoissondistribution when callingstd::lgammainsidecalculate_poisson_probabilitiesthere was a race condition. This is happening becausestd::lgammastores the sign of the output inside a global variable. To fix this we can uselgamma_rwhich allows us to store the sign locally without storing it in a global variable.Since
lgamma_ris not part of thestdliband is only implemented inglibc, it is not available for windows (msvc). This is the reason why we need to check if we are currently on windows or not.Test Plan
Standard CI test suite. Also local testing on both linux and windows.
Also re-run with
TSANto see if race condition is still reported.Test Result
Local unit test ran on linux and windows shows no failures.
TSANrun shows no race condition originated from rocrand.Submission Checklist