-
Notifications
You must be signed in to change notification settings - Fork 23
Open Boundaries #666
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
Open Boundaries #666
Conversation
MaxThevenet
left a comment
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 this PR! See minor comments inlined. Besides,
- Could you see with @SeverinDiederichs to add the offset beam in vacuum test in CI?
- Could you share profiling data for 1 ppc and a transverse box size of 2048^2?
| // ignore everything outside of 95% the min radius as the Taylor expansion only converges | ||
| // outside of a circular patch containing the sources, i.e. the sources can't be further | ||
| // from the center than the closest boundary as it would be the case in the corners | ||
| const amrex::Real cutoff_sq = pow<2>(0.95_rt * radius * scale); |
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.
Isn't 95% arbitrary? Should it be e.g. min radius minus 3 cells or so?
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.
It’s not about a fixed number of Cells, rather it’s to stay a safe distance away from the convergence radius of the Taylor expansion.
| /** \brief calculate low integer powers base^exp | ||
| * \param[in] base base of power | ||
| */ | ||
| template<unsigned int exp> AMREX_GPU_HOST_DEVICE AMREX_FORCE_INLINE | ||
| amrex::Real pow (amrex::Real base) { | ||
| using namespace amrex::literals; | ||
| if constexpr (exp==0) { | ||
| return 1._rt; | ||
| } else if constexpr (exp==1) { | ||
| return base; | ||
| } else { | ||
| return pow<exp-1>(base) * base; | ||
| } | ||
| return 0._rt; //shut up compiler | ||
| } |
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.
std::pow is indeed much slower than multiplications for integer powers. @atmyers could that go to AMReX?
| * ``fields.extended_solve`` (`bool`) optional (default `0`) | ||
| Extends the area of the FFT Poisson solver to the ghost cells. This can reduce artefacts | ||
| originating from the boundary for long simulations. | ||
|
|
||
| * ``fields.open_boundary`` (`bool`) optional (default `0`) |
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 for future extensibility, you could use the same syntax as in WarpX here: boundary.field_lo and boundary.field_hi
https://warpx.readthedocs.io/en/latest/usage/parameters.html#domain-boundary-conditions
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.
No, because right now there is now functionality to have different boundary conditions for different sides of the box (and no use in hipace?). Moreover, as I understand it, the four combinations of fields.extended_solve = true/false and fields.open_boundary = true/false would require four differently name boundary conditions with that syntax.
…into change_numerics
MaxThevenet
left a comment
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, looking forward to testing it!
SeverinDiederichs
left a comment
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.
Great work! Thanks for this addition


based on #723, #724 and #725
To enable Open Boundaries use:
The Poisson equations are solved with open boundaries rather than zero boundaries by integrating a Taylor expansion of the Greens function of the transverse laplacien with the source and using that as boundary conditions of the FFT Poisson solver. Note that this does not apply to the Helmholtz multigrid solver.
Old: development
New: Open Boundary
Performance:
amr.n_cell = 1019 1019 2000andplasma.ppc = 2 2Open Boundaries are calculated in
Fields::SetBoundaryCondition()amr.n_cell = 2043 2043 3000andplasma.ppc = 1 1: