Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

lockAdd: case of 2D plane in 3D #3700

Merged
merged 1 commit into from
Jan 22, 2024

Conversation

WeiqunZhang
Copy link
Member

Summary

In HiPACE++, atomicAdd is used on 2d x & y planes even though AMREX_SPACEDIM is 3. In that case, we would have all threads competing for a single lock in the previous implementation of lockAdd. This PR fixes this use case by having locks associated with the y-direction when the number of cells in the z-direction is 1.

Additional background

Hi-PACE/hipace#1059

Checklist

The proposed changes:

  • fix a bug or incorrect behavior in AMReX
  • add new capabilities to AMReX
  • changes answers in the test suite to more than roundoff level
  • are likely to significantly affect the results of downstream AMReX users
  • include documentation in the code and/or rst files, if appropriate

In HiPACE++, atomicAdd is used on 2d x & y planes even though AMREX_SPACEDIM
is 3. In that case, we would have all threads competing for a single lock in
the previous implementation of lockAdd. This PR fixes this use case by
having locks associated with the y-direction when the number of cells in the
z-direction is 1.
Copy link
Member

@AlexanderSinn AlexanderSinn left a comment

Choose a reason for hiding this comment

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

I tested HiPACE++ running on two 48-core CPUs with 4095*4095*100 cells and a tile size of 256^2. While the lockAdd version from this PR is faster, particularly at lower thread counts, it does not solve the bad scaling past 48 threads.
image

@WeiqunZhang WeiqunZhang merged commit 0c59bad into AMReX-Codes:development Jan 22, 2024
69 checks passed
@WeiqunZhang WeiqunZhang deleted the lockadd2 branch January 22, 2024 23:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants