Skip to content

fix omp scan bug#6560

Merged
bernhardmgruber merged 17 commits intoNVIDIA:mainfrom
charan-003:fix_omp_scan_bug
Nov 20, 2025
Merged

fix omp scan bug#6560
bernhardmgruber merged 17 commits intoNVIDIA:mainfrom
charan-003:fix_omp_scan_bug

Conversation

@charan-003
Copy link
Contributor

using the first element of each block as the init value

closes #6317

@charan-003 charan-003 requested a review from a team as a code owner November 9, 2025 09:05
@charan-003 charan-003 requested a review from davebayer November 9, 2025 09:05
@github-project-automation github-project-automation bot moved this to Todo in CCCL Nov 9, 2025
@copy-pr-bot
Copy link
Contributor

copy-pr-bot bot commented Nov 9, 2025

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

@cccl-authenticator-app cccl-authenticator-app bot moved this from Todo to In Review in CCCL Nov 9, 2025
@github-project-automation github-project-automation bot moved this from In Review to In Progress in CCCL Nov 10, 2025
@bernhardmgruber
Copy link
Contributor

@charan-003 I added a few more tests.

@bernhardmgruber
Copy link
Contributor

/ok to test ae2c2c7

@github-actions

This comment has been minimized.

@bernhardmgruber
Copy link
Contributor

/ok to test f549d50

@github-actions

This comment has been minimized.

@bernhardmgruber
Copy link
Contributor

/ok to test b0d8c74

@github-actions

This comment has been minimized.

@bernhardmgruber
Copy link
Contributor

/ok to test 9ea6902

@github-actions

This comment has been minimized.

@bernhardmgruber
Copy link
Contributor

@charan-003 I will review this next week.

Copy link
Contributor

@bernhardmgruber bernhardmgruber left a comment

Choose a reason for hiding this comment

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

This is looking good! A few nits, otherwise LGTM!

@bernhardmgruber
Copy link
Contributor

/ok to test 660b6f2

@bernhardmgruber
Copy link
Contributor

/ok to test 9f904ea

@charan-003
Copy link
Contributor Author

@bernhardmgruber I was wondering if it's possible to perform CI tests locally without using Docker?

@bernhardmgruber
Copy link
Contributor

@bernhardmgruber I was wondering if it's possible to perform CI tests locally without using Docker?

Sure! CCCL is just an ordinary cmake project so you can:

mkdir my_build
cd my_build
cmake .. -DTHRUST_ENABLE_MULTICONFIG=OFF
ccmake . # enable what you need, for example Thrust. make sure to set the THRUST_DEVICE_SYSTEM=OMP
make thrust.test.scan
./bin/thrust.test.scan

@github-actions

This comment has been minimized.

@bernhardmgruber
Copy link
Contributor

/ok to test a8df5c0

@bernhardmgruber
Copy link
Contributor

/ok to test 9a7675d

@github-actions

This comment has been minimized.

@bernhardmgruber
Copy link
Contributor

Thank you so much for taking the time to push the fix through!

@bernhardmgruber
Copy link
Contributor

@dkolsen-pgi please re-review and unblock. Thx!

{
::cuda::std::exclusive_scan(block_sums.begin(), block_sums.end(), block_sums.begin(), accum_t{}, wrapped_binary_op);
// For no init inclusive scan: exclusive_scan starting at second element with first element as init
if (num_threads > 1)
Copy link
Contributor

Choose a reason for hiding this comment

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

You added the assertion as suggested, but forgot to remove this now unnecessary check for num_threads > 1.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Right, thanks for bringing this up.

@bernhardmgruber
Copy link
Contributor

/ok to test 0b1e611

@bernhardmgruber bernhardmgruber enabled auto-merge (squash) November 20, 2025 00:05
Copy link
Contributor

@dkolsen-pgi dkolsen-pgi left a comment

Choose a reason for hiding this comment

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

I only looked at scan.h, not at the tests. That file now looks good.

@github-project-automation github-project-automation bot moved this from In Progress to In Review in CCCL Nov 20, 2025
@github-actions
Copy link
Contributor

🥳 CI Workflow Results

🟩 Finished in 1h 30m: Pass: 100%/84 | Total: 14h 28m | Max: 42m 40s | Hits: 88%/116157

See results here.

@bernhardmgruber bernhardmgruber merged commit bb8a5b9 into NVIDIA:main Nov 20, 2025
97 of 98 checks passed
@github-project-automation github-project-automation bot moved this from In Review to Done in CCCL Nov 20, 2025
github-actions bot pushed a commit that referenced this pull request Nov 20, 2025
Co-authored-by: Bernhard Manfred Gruber <bernhardmgruber@gmail.com>
(cherry picked from commit bb8a5b9)
@github-actions
Copy link
Contributor

Successfully created backport PR for branch/3.2.x:

elstehle added a commit that referenced this pull request Nov 20, 2025
(cherry picked from commit bb8a5b9)

Co-authored-by: charan-003 <yadavcharan003@gmail.com>
Co-authored-by: Bernhard Manfred Gruber <bernhardmgruber@gmail.com>
Co-authored-by: Elias Stehle <3958403+elstehle@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

[BUG]: stdpar multicore scan tests fail at runtime after commit 1238f287f4f654bff12e4d4a29b7dfa1c40a12fe

3 participants