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

Add MultiFab::sum(region) and sum_unique(region) #3871

Merged
merged 4 commits into from
Apr 10, 2024

Conversation

WeiqunZhang
Copy link
Member

Summary

Add two new functions to MultiFab that return the sum of the given region.

Additional background

#3869

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

Add two new functions to MultiFab that return the sum of the given region.
@asalmgren
Copy link
Member

  1. duplicatly is misspelled
  2. I'm worried about people assuming this works correctly with periodicity even though the code has a comment that it doesn't account for that

@WeiqunZhang
Copy link
Member Author

Yes, there could be potential confusion even though logically speaking it should not because without being provided the periodicity information amrex cannot know how to handle it. So the caller has to assume periodicity is not part of the consideration. This is a feature request from WarpX, which does not need the periodic boundaries to be handled in this function. I didn't try to add periodicity support because it's actually quite awkward to implement. We would need to memcpy a vector of periodically shifted Boxes to device in order to do all these in one kernel. Maybe we can wait till someone wants the feature.

@ax3l ax3l requested a review from RemiLehe April 4, 2024 22:21
WeiqunZhang and others added 2 commits April 5, 2024 09:22
Co-authored-by: Axel Huebl <axel.huebl@plasma.ninja>
Copy link
Member

@ax3l ax3l left a comment

Choose a reason for hiding this comment

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

Thank you! :)

@WeiqunZhang WeiqunZhang merged commit 6c62475 into AMReX-Codes:development Apr 10, 2024
68 of 69 checks passed
@WeiqunZhang WeiqunZhang deleted the sum_region branch April 10, 2024 23:12
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.

3 participants