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

Staircased embedded boundaries in the YEE solver #1881

Merged
merged 38 commits into from
Apr 27, 2021
Merged

Staircased embedded boundaries in the YEE solver #1881

merged 38 commits into from
Apr 27, 2021

Conversation

lgiacome
Copy link
Member

@lgiacome lgiacome commented Apr 7, 2021

Adding embedded boundaries with staircasing approximation in the YEE solver.

The changes include:

  • reading geometrical data from AmREX (i.e. partial length of cut edges and partial areas of cut faces)
  • modification of the Yee solver to do the field update only when a cell is at least partially outside of the conductor
  • adding the possibility to initialize the analytic electromagnetic field in a spherical resonating cavity (useful for testing)
  • adding the test case of a spherical resonating cavity

There is a small issue with the code implemented in PR #1641. When I compile with the Embedded Boundary option and I don't specify any conductor in the input file I get the following error message
amrex::Abort::0::geom_type 1 not supported !!!
while the geometry should be interpreted as all_regular.
This issue is now fixed by this PR

@lgiacome lgiacome changed the title Staircased embedded boundaries in the YEE solver [WIP] Staircased embedded boundaries in the YEE solver Apr 7, 2021
@lgiacome lgiacome closed this Apr 7, 2021
amrex::Real H_theta = 0;
amrex::Real mu_r = PhysConst::mu0;
#ifdef AMREX_USE_EB
amrex::Real H_phi = k / (r_sphere * mu_r) * boost::math::sph_bessel(1, k*r) * sin(theta) * cos(k * PhysConst::c * t);
Copy link
Member

@ax3l ax3l Apr 7, 2021

Choose a reason for hiding this comment

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

Discussed on Slack:

yes we do not depend on boost because it's a pretty heavy dependency to pull in. But if you only need them on the host-side, you can take the C++17 functions!
https://en.cppreference.com/w/cpp/numeric/special_functions/sph_bessel

We are compiling with C++14 or newer, but we can with no problem require C++17 for selected features already! (We will probably transition to require C++17 anyway during the year.)

So it would be no problem to say e.g. "if you want to use embedded boundaries, use a more recent compiler", absolutely no problem.

@lgiacome lgiacome reopened this Apr 8, 2021
@lgiacome lgiacome changed the title [WIP] Staircased embedded boundaries in the YEE solver Staircased embedded boundaries in the YEE solver Apr 8, 2021
@ax3l ax3l self-assigned this Apr 8, 2021
@ax3l ax3l requested a review from dpgrote April 8, 2021 18:54
@ax3l ax3l added component: boundary PML, embedded boundaries, et al. enhancement New feature or request labels Apr 8, 2021
@ax3l ax3l requested a review from WeiqunZhang April 8, 2021 18:55
amrex::Real H_r = 0;
amrex::Real H_theta = 0;
amrex::Real mu_r = PhysConst::mu0;
amrex::Real H_phi = k / (r_sphere * mu_r) * std::sph_bessel(1, k*r) * sin(theta) * cos(k * PhysConst::c * t);
Copy link
Member

@ax3l ax3l Apr 8, 2021

Choose a reason for hiding this comment

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

We decided to already require C++17 for EB features (instead of pulling in external dependencies).
memo to myself: reflect that in build systems and add checks at compile time asserting __cplusplus version.

I opened a feature request to Nvidia about future support of spherical bessel functions on device.

@RemiLehe RemiLehe requested review from jlvay and RemiLehe and removed request for jlvay April 19, 2021 22:09
@RemiLehe
Copy link
Member

As discussed offline with @lgiacome, the automated tests will be slightly modified to consider a cubic conducting boundary, instead of a spherical one. This will make it possible to initialize the fields with the parser, and will also remove the need to compute the spherical Bessel function.

Thanks again for this PR @lgiacome !

@RemiLehe RemiLehe changed the title Staircased embedded boundaries in the YEE solver [WIP] Staircased embedded boundaries in the YEE solver Apr 21, 2021
@RemiLehe RemiLehe changed the title [WIP] Staircased embedded boundaries in the YEE solver Staircased embedded boundaries in the YEE solver Apr 23, 2021
Copy link
Member

@RemiLehe RemiLehe left a comment

Choose a reason for hiding this comment

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

Thanks for updating the tests! This looks very good.
I have a few more comments (see inline comments).

ds = yt.load(filename)
data = ds.covering_grid(level=0, left_edge=ds.domain_left_edge, dims=ds.domain_dimensions)

tol_err = 1e-5
Copy link
Member

Choose a reason for hiding this comment

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

My understanding is that this value is probably too high for this test to properly check the simulated fields.

More specifically, if I didn't make any mistake, I think even if Bx_sim/By_sim/Bz_sim were completely wrong (e.g. Bx_sim = By_sim = Bz_sim = 0), then the err_x/err_y/err_z would be on the order of 1.e-7 or 1.e-6, i.e. below tol_err, which means that the test would pass.

One could use a more stringent value for tol_err.
Or one alternative maybe is to use a relative error like this:

rel_tol_err = 0.1
Bx_sim = data['Bx'].to_ndarray()
rel_err_x = np.sqrt( np.sum(np.square(Bx_sim - Bx_th)) / np.sum(np.square(Bx_th)) )
assert(rel_err_x < rel_tol_err )

Copy link
Member Author

Choose a reason for hiding this comment

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

Good point! The relative error you are suggesting seems good to me, I will implement it. Thanks a lot for pointing out this.

Copy link
Member Author

Choose a reason for hiding this comment

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

I have implemented the relative error. By the way, I am not checking the error on Bx anymore as it is zero everywhere and the relative error would blow up.

Source/EmbeddedBoundary/WarpXInitEB.cpp Outdated Show resolved Hide resolved
@RemiLehe
Copy link
Member

RemiLehe commented Apr 26, 2021

Thanks again for updating the test. In case this helps (e.g. for future reference to this PR), I made a small movie of the test, which shows that the field is only updated inside the volume surrounded by embedded boundaries (in this particular case: a square that goes from -0.5 to 0.5 in each direction)
movie

@lgtm-com
Copy link
Contributor

lgtm-com bot commented Apr 27, 2021

This pull request introduces 1 alert when merging 35705d0 into 5035351 - view on LGTM.com

new alerts:

  • 1 for Unused import

@RemiLehe
Copy link
Member

Thanks for incorporating the requested changes! 🚀

@RemiLehe RemiLehe merged commit e92cb5c into ECP-WarpX:development Apr 27, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: boundary PML, embedded boundaries, et al. enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants