Merge All Three PPM Kernels Into One #392
Open
Conversation
This file contains 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
This change makes it clearer that the file contains some of the internal workings of the reconstructors and not the reconstructors themselves. It also clears the way for a new reconstruction.h file that can contain the controller function for the future device function version of the reconstructors.
It's not used anywhere yet but will be used in the fused reconstruction
I moved the reconstruction::Primitive struct into the hydro utilities. Along with that I added a struct for conserved variables and a small struct for vector quantities that just has members x, y, and z to make storage and movement of vectors easier. I also made the primitive and conserved structs use that vector struct for their vector quantites. Once that was done I fixed all the references to reconstructon::Primitive to point at the new location/version of the struct, updated it's usage, and updated all the relevant tests.
Renamed to gas_energy_specific to make usage clearer
Refactor the Exact Riemann solver to use the new InterfaceState struct in prep for reconstruction refactor
Refactor the Roe Riemann solver to use the new InterfaceState struct in prep for reconstruction refactor
Refactor the HLL Riemann solver to use the new InterfaceState struct in prep for reconstruction refactor
Refactor the HLLC Riemann solver to use the new InterfaceState struct in prep for reconstruction refactor
This constructor maens that when the struct is initialized in tests there is no ABI dependence and instead the internals can be easily changed without effecting the interface. Also, fixed some tests that had bugs due to their ABI dependency.
Refactor the HLLC Riemann solver to use the new `InterfaceState` struct in prep for reconstruction refactor. This required a bit more work as there than the other Riemann solvers since I'm replacing the existing mhd::internal::State struct rather than replacing a bunch of scalar variables. To do this I also added a constructor to `reconstruction::InterfaceState` to eliminate the previous ABI dependency on `mhd::internal::State`
This was to avoid a circular dependency between the mhd and hydro util files. Now this fundemental structs are on their own and can be included as needed Fix typo in Conserved struct
Renamed to indicate that they are the specific quantities
Some variables were accidentally misnamed and there was a typo in part of VL_3D related to the exact solver. Both issues have been fixed
Now it's consistent with Compute_Slope
This was done to avoid confusing with std::vector and DeviceVector
This version supports indexing rather than just accessing the xyz members directly. Matthew Abruzzo provided the new version of the struct, Robert Caddy added doxygen comments and implemented it in Cholla. Co-authored-by: Matthew Abruzzo <matthewabruzzo@gmail.com>
Some variables were accidentally misnamed and there was a typo in part of VL_3D related to the exact solver. Both issues have been fixed
Added functions for cyclically permutting a hydro_utililties::Vector. One function permutes it once, the second permutes it twice
hydro_utilities::Load_Cell_Conserved loads all the data from a cell, uses cell centered magnetic fields
Refactored this function to put the limiter into its own function, rename it to better reflect what it actually does, and move the characteristic to primitive conversion outside the function
Sound speed plays a similar role to the other members of EigenVecs so including it makes sense rather than passing around extra arguments
Doxygen for PCM, HLLD, and Reconstruction functions. Make all `direction` tparams of type uint instead of a mix of int, size_t, and uint. uint was chosen since direction should never be negative and size_t is misleading in that it looks like it should be an array index.
These don't change much for most builds but they do allow higher order reconstructions to be fused later if needed
This is the first step in combining the two PLM methods into one with a switch for the limiting variables. Currently that switch exists but the kernels haven't been modified to use it yet.
- Removed the PLMP kernel since PLMC can do both now - Renamed PLMC_cuda to PLM_cuda since it can now do limiting in either set of variables - Renamed plmc files to plm - The PLM kernel has MHD support for either type of limiting
Moved the computation of the eigenvectors so it's not unnecerarily done in PLMP
Refactored this function to put the limiter into its own function, rename it to better reflect what it actually does, and move the characteristic to primitive conversion outside the function
Sound speed plays a similar role to the other members of EigenVecs so including it makes sense rather than passing around extra arguments
I removed PPMC_CTU and added a call to the characteristic tracing function to PPMC_VL and renamed it to PPMC_cuda
bcaddy
changed the title
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.
Summary
This merges the PPMC_CTU, PPMC_VL, and PPMP kernels into one single kernel.
PPMC/PPMP Merge
Accuracy & Stability
Similar to PR #391 I ran as suite of tests to verify stability and correctness. PPM is generally more oscillatory than PLM and the implosion test is not working either before or after this refactor; in fact it's not working on dev either.
mhd_ppm_tests.zip
hydro_ppm_tests.zip
Dependencies
Depends on PRs #371, #375, #377, and #391. Until those are merged this will look like it has a lot more changes than it does. A version without all those extra changes can be found here.