-
Notifications
You must be signed in to change notification settings - Fork 201
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
Use RK4 to integrate the B-field in time in the hybrid-PIC algorithm #4461
Use RK4 to integrate the B-field in time in the hybrid-PIC algorithm #4461
Conversation
e110406
to
863f44a
Compare
…lgorithm Co-authored-by: Avigdor Veksler <[email protected]>
* continued transition to RK4 in B-field time integration in hybrid-PIC algorithm * RK-integration over all levels
for more information, see https://pre-commit.ci
1910fc1
to
ff0c9c0
Compare
Source/FieldSolver/FiniteDifferenceSolver/HybridPICModel/HybridPICModel.cpp
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this looks good and I am excited to use it as well since the previous second order integration scheme needed substantial subcycling to remain stable.
Source/FieldSolver/FiniteDifferenceSolver/HybridPICModel/HybridPICModel.cpp
Outdated
Show resolved
Hide resolved
Source/FieldSolver/FiniteDifferenceSolver/HybridPICModel/HybridPICModel.cpp
Outdated
Show resolved
Hide resolved
Source/FieldSolver/FiniteDifferenceSolver/HybridPICModel/HybridPICModel.cpp
Outdated
Show resolved
Hide resolved
B_old[ii] = MultiFab( | ||
Bfield[lev][ii]->boxArray(), Bfield[lev][ii]->DistributionMap(), 1, | ||
Bfield[lev][ii]->nGrowVect() | ||
); | ||
MultiFab::Copy(B_old[ii], *Bfield[lev][ii], 0, 0, 1, ng); | ||
|
||
K[ii] = MultiFab( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If I am not mistaken, then you can reduce the memory footprint of the currently allocated MultiFabs if you destruct the prior stored on before constructing the next one... cc @WeiqunZhang
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
K[ii]
is empty before the assignment. In some other situations, yes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ax3l Just to make sure I understand, you mean if K[ii]
already held a MultiFab it would be better to destruct it before overwriting with a new MultiFab?
Co-authored-by: Axel Huebl <[email protected]>
amrex::Vector<std::unique_ptr<amrex::MultiFab>> const& rhofield, | ||
amrex::Vector<std::array< std::unique_ptr<amrex::MultiFab>, 3>> const& edge_lengths, | ||
amrex::Real dt, DtType dt_type, | ||
amrex::IntVect ng, std::optional<bool> nodal_sync); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you add docstrings for these functions (esp. for the parameter dt_type
, which may be the least intuitive)?
// Subtract B_old from the Bfield for each direction, to get | ||
// B = dt * K2 + 0.5 * dt * K3. | ||
MultiFab::Subtract(*Bfield[lev][ii], B_old[ii], 0, 0, 1, ng); | ||
|
||
// Add dt * K2 + 0.5 * dt * K3 to index 0 of K (= 0.5 * dt * K0). | ||
MultiFab::Add(K[ii], *Bfield[lev][ii], 0, 0, 1, ng); | ||
|
||
// Add 2 * 0.5 * dt * K1 to index 0 of K. | ||
MultiFab::LinComb( | ||
K[ii], 1.0, K[ii], 0, 2.0, K[ii], 1, 0, 1, ng | ||
); | ||
|
||
// Overwrite the Bfield with the Runge-Kutta sum: | ||
// B_new = B_old + 1/3 * dt * (0.5 * K0 + K1 + K2 + 0.5 * K3). | ||
MultiFab::LinComb( | ||
*Bfield[lev][ii], 1.0, B_old[ii], 0, 1.0/3.0, K[ii], 0, 0, 1, ng | ||
); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would it be worth to write all these operations in a single ParallelFor
kernel, both for readability and to avoid kernel-launch overheads from multiple kernels?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the suggestion. I also thought about that as a good follow-up performance improvement but haven't gotten around to it yet since with this new scheme the field solver takes a small amount of runtime in the simulations we are commonly running (so it is not high on the priority list). But of course it would be a good thing to make the code more performant. My current idea is to save this for a time when we have a new team member for which this would be a good task as a way to learn the hybrid-PIC algorithm as well as how to work with WarpX/AMReX.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me! Thanks for this PR!
I added a few comments that could be addressed in follow-up PRs.
This PR provides a performance improvement in the hybrid-PIC algorithm by using a Runge-Kutta scheme to advance the B-field rather than a simple forward differencing scheme. This allows a lower number of
substeps
to be used in the field advance which improves the algorithm performance.This should be merged after #4405 as these changes were branched off that PR.
Todo: