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

Energy Conserving P2 Can-pb Colls Fix #491

Merged
merged 16 commits into from
Oct 10, 2024
Merged

Conversation

johnson452
Copy link
Collaborator

@johnson452 johnson452 commented Oct 5, 2024

See the detailed design review: Here.

PR Summary:

  • Fixes energy preservation issues associated with can-pb P2 in curved space. (See DR for details including written up equations).
  • Fixes issues in main associated with the GPU build (Jimmy).

Verified:
[X] Builds and runs on GPUs
[X] CPU tests are valgrind clean

johnson452 and others added 15 commits September 14, 2024 01:01
…integrated moment only outputs the integrated energy
…hich means main does not build on GPUs. After some digging, I think this bug appeared when the wv_eqn functions were modified to uniformly take the wv_eqn object (so things like the mixture formalism know how many components to rotate). I think when this change occurred that introduced too many indirections on GPUs and the GPU started throwing the nvlink errors we've seen before that always arise when the GPU has completely crapped out on handling the layering Gkeyll does. Testing to see if removing a layer of indirection helps.
… inside the equation object, we just directly call the function in the private header for the individual wv_eqn objects. Please for the love of CUDA gods work
…hods now because we can call them from both host and device code
… to clean it up and prevent lots of unused variable warnings (since now this header file is being directly utilized by kernels).
Copy link
Owner

@ammarhakim ammarhakim left a comment

Choose a reason for hiding this comment

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

Ready to merge

@JonathanGorard JonathanGorard merged commit 747d063 into main Oct 10, 2024
2 checks passed
@JunoRavin JunoRavin deleted the can_pb_total_energy_dynvec branch October 16, 2024 13:54
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.

4 participants