-
Notifications
You must be signed in to change notification settings - Fork 199
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 new stair-case approximation in hybrid solver #5558
Conversation
…to move_stair_case_approx
This reverts commit 4fd4dc7.
edge_lengths
and face_areas
anymoreWhen updating `E` (and when initializing the field values for `E` and `B` with a parser), we used to check somewhat complicated conditions that relied on the MultiFabs `edge_lengths` and `face_areas`. This PR introduces separate arrays (`m_eb_update_E` and `m_eb_update_B`, which are `iMultiFab` instead of `MultiFab` and thus take up much less memory). These arrays contain flags that keep track of where to udpate the fields (i.e. the black crosses in the above figure). These arrays are initialized in separate functions which keep the same EB definition. (PR #5534 will then change this definition.) The code for the field pusher and field initialization uses these arrays (instead of edge_lengths and face_areas. It is thus significantly simpler and avoids duplicating complicated if conditions. The above changes have not yet been implemented for the hybrid solver. This will instead be done in a separate PR: #5558. Once this is complete, the MultiFab `edge_lengths` and `face_areas` will not be needed anymore (except for the ECT solver), and we will thus only allocate them for the ECT solver. This should save a significant amount of memory. --------- Co-authored-by: Roelof Groenewald <[email protected]>
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.
This looks good. I'll just give it a final look through once #5534 is merged and any conflicts are resolved before approving.
Co-authored-by: Roelof Groenewald <[email protected]>
for more information, see https://pre-commit.ci
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.
Just a few more comments for typos I spotted.
Source/FieldSolver/FiniteDifferenceSolver/FiniteDifferenceSolver.H
Outdated
Show resolved
Hide resolved
Source/FieldSolver/FiniteDifferenceSolver/FiniteDifferenceSolver.H
Outdated
Show resolved
Hide resolved
Source/WarpX.H
Outdated
@@ -927,8 +927,7 @@ public: | |||
* \param[in] fx_parser parser function to initialize x-field | |||
* \param[in] fy_parser parser function to initialize y-field | |||
* \param[in] fz_parser parser function to initialize z-field | |||
* \param[in] edge_lengths edge lengths information | |||
* \param[in] face_areas face areas information | |||
* \param[in] eb_update_field flag indicating which gridpoints should be modified by this functions |
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.
The topology
parameter description below this line can be removed.
Co-authored-by: Roelof Groenewald <[email protected]>
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!
Merge #5534 first.
This extends the changes of #5534 and #5574 to the hybrid solver.
Note that this effectively changes the definition of the stair-cased embedded boundary for the hybrid solver.