Add `PECInsulator` boundary condition #4943

dpgrote · 2024-05-21T23:30:10Z

This PR adds a mixed PEC and insulator boundary condition. This allows an insulator to be placed on a portion of the boundary. The rest of that boundary will be PEC. Within the insulator portion, the tangential fields can be specified on the boundary (as functions of space and time). The normal fields and fields not specified are extrapolated to the guard cells from the valid cells. The fields are specified in pairs, the two tangential electric fields, and the two tangential magnetic fields. In each pair, if one is set, the other will be zeroed if not set.

A use case is the simulation of a dynamic pinch, driven by an external current, represented as a time dependent B field on the boundary.

PECinsulatorBC_warpX_summaryOnly.pdf

for more information, see https://pre-commit.ci

EZoni · 2024-10-08T18:49:47Z

@RevathiJambunathan @roelof-groenewald
I added you as reviewers of this PR as well, since you did extensive work on PEC boundary conditions in WarpX in the past.

Docs/source/usage/parameters.rst

Source/BoundaryConditions/PEC_Insulator.H

EZoni · 2024-10-08T20:59:55Z

Source/BoundaryConditions/PEC_Insulator.H

+     * \param[in,out] Efield
+     * \param[in]     ng_fieldgather      number of guard cells used by field gather
+     * \param[in]     geom                geometry object of level "lev"
+     * \param[in]     lev                 level of the Multifab
+     * \param[in]     patch_type          coarse or fine
+     * \param[in]     ref_ratios          vector containing the refinement ratios of the refinement levels
+     * \param[in]     time                current time of the simulation
+     * \param[in]     split_pml_field     whether pml the multifab is the regular Efield or


Could you add the docstrings for field_boundary_lo and field_boundary_hi as well?

EZoni · 2024-10-08T21:00:36Z

Source/BoundaryConditions/PEC_Insulator.H

+     * \param[in,out] field
+     * \param[in]     ng_fieldgather      number of guard cells used by field gather
+     * \param[in]     geom                geometry object of level "lev"
+     * \param[in]     lev                 level of the Multifab
+     * \param[in]     patch_type          coarse or fine
+     * \param[in]     ref_ratios          vector containing the refinement ratios of the refinement levels
+     * \param[in]     time                current time of the simulation
+     * \param[in]     split_pml_field     whether pml the multifab is the regular Efield or
+     *                                    split pml field
+     * \param[in]     E_like whether the field is E like or B like
+     * \param[in]     set_F_x_lo whether the tangential field at the boundary was specified
+     * \param[in]     set_F_x_hi whether the tangential field at the boundary was specified
+     * \param[in]     a_Fy_x_lo the parser for the tangential field at the boundary
+     * \param[in]     a_Fz_x_lo the parser for the tangential field at the boundary
+     * \param[in]     a_Fy_x_hi the parser for the tangential field at the boundary
+     * \param[in]     a_Fz_x_hi the parser for the tangential field at the boundary
+     * \param[in]     set_F_y_lo whether the tangential field at the boundary was specified
+     * \param[in]     set_F_y_hi whether the tangential field at the boundary was specified
+     * \param[in]     a_Fx_y_lo the parser for the tangential field at the boundary
+     * \param[in]     a_Fz_y_lo the parser for the tangential field at the boundary
+     * \param[in]     a_Fx_y_hi the parser for the tangential field at the boundary
+     * \param[in]     a_Fz_y_hi the parser for the tangential field at the boundary
+     * \param[in]     set_F_z_lo whether the tangential field at the boundary was specified
+     * \param[in]     set_F_z_hi whether the tangential field at the boundary was specified
+     * \param[in]     a_Fx_z_lo the parser for the tangential field at the boundary
+     * \param[in]     a_Fy_z_lo the parser for the tangential field at the boundary
+     * \param[in]     a_Fx_z_hi the parser for the tangential field at the boundary
+     * \param[in]     a_Fy_z_hi the parser for the tangential field at the boundary


Could you add the docstrings for field_boundary_lo and field_boundary_hi as well?

EZoni · 2024-10-08T21:08:45Z

Source/BoundaryConditions/PEC_Insulator.H

+    void ApplyPEC_InsulatortoBfield (std::array<amrex::MultiFab*, 3> Bfield,
+                                     amrex::Array<FieldBoundaryType,AMREX_SPACEDIM> const & field_boundary_lo,
+                                     amrex::Array<FieldBoundaryType,AMREX_SPACEDIM> const & field_boundary_hi,
+                                     amrex::IntVect const & ng_fieldgather, amrex::Geometry const & geom,
+                                     int lev, PatchType patch_type, amrex::Vector<amrex::IntVect> const & ref_ratios,
+                                     amrex::Real time);


Why don't we need a split_pml_field argument for B, as for E? In other words, why is the splitting of B, which I think occurs in the PML, not relevant in this case?

Good question. I copied this pattern from the PEC boundary conditions. Do you know why the split is done there?

Ah interesting, I don't know. I think this would be a question for @RevathiJambunathan or @roelof-groenewald.

Pinging @RevathiJambunathan and @roelof-groenewald just to make sure that the PR doesn't get stalled.

Re-Pinging @RevathiJambunathan and @roelof-groenewald.

Sorry for the slow response. It looks like the flag was introduced in #2541 to fix an observed bug when one boundary is PEC and an adjacent one is PML. @RevathiJambunathan would know better but I expect it is not needed to apply the same to the B-field since the update to the E-field ensures that the B-field values come out appropriately in the B-field push.

Source/BoundaryConditions/PEC_Insulator.cpp

EZoni · 2024-10-08T21:18:25Z

Source/BoundaryConditions/PEC_Insulator.cpp

+                            // Upper radial boundary
+                            amrex::Real const rguard = ijk_vec[idim] + 0.5_rt*(1._rt - is_nodal[idim]);
+                            if (icomp == 0) {
+                                // Add radial scale so that drFr/dr = 0.


What is the formula in this inline comment? Is it $dF_r/dr=0$ or $d(rF_r)/dr=0$? In the second case I would maybe write

Suggested change

// Add radial scale so that drFr/dr = 0.

// Add radial scale so that d(rF_r)/dr = 0.

or at least

Suggested change

// Add radial scale so that drFr/dr = 0.

// Add radial scale so that d(rFr)/dr = 0.

EZoni · 2024-10-08T21:36:15Z

Examples/Tests/pec/CMakeLists.txt

+add_warpx_test(
+    test_2d_pec_field_insulator  # name
+    2  # dims
+    2  # nprocs
+    inputs_test_2d_pec_field_insulator  # inputs
+    analysis_default_regression.py  # analysis
+    diags/diag1000010  # output
+    OFF  # dependency
+)


Given the complexity of the new feature, is there a physics analysis that can be done, beyond the checksums analysis?

Also, is this intentionally tested only in 2D? The implementation seems to be general for all dimensions, so maybe it could be worth adding tests for those as well. What do you think?

This test was copied from the test case for the same boundary condition in the picnic code. That code's primary use is 2D. I don't know of any physics analysis that could be done beyond checking that the fields at the boundary are as expected. I could add other cases that do that simple check.

Fixes in documentation Co-authored-by: Edoardo Zoni <[email protected]>

EZoni · 2024-11-04T17:55:21Z

@dpgrote

Sorry that this got stalled for so long. I was waiting for feedback from other developers on some of the questions I asked in my review, but if this is working for you we can go ahead and merge as is. There seems to be conflicts now unfortunately, so those would have to be fixed first.

dpgrote · 2024-11-04T18:24:01Z

@dpgrote

Sorry that this got stalled for so long. I was waiting for feedback from other developers on some of the questions I asked in my review, but if this is working for you we can go ahead and merge as is. There seems to be conflicts now unfortunately, so those would have to be fixed first.

No problem. Though something weird has happened to the commit history. There are a number of commits from the development branch listed here but with different hash tags. I'll see if I can clean this up. Otherwise I'll have to copy the changes to a different branch and submit a new PR.

dpgrote · 2024-11-04T18:33:47Z

@dpgrote
Sorry that this got stalled for so long. I was waiting for feedback from other developers on some of the questions I asked in my review, but if this is working for you we can go ahead and merge as is. There seems to be conflicts now unfortunately, so those would have to be fixed first.

No problem. Though something weird has happened to the commit history. There are a number of commits from the development branch listed here but with different hash tags. I'll see if I can clean this up. Otherwise I'll have to copy the changes to a different branch and submit a new PR.

Ok, that worked. I locally removed all of the duplicated commits, redid the merge, and then did a force push.

JustinRayAngus

This looks good to me. Thanks for the PR Dave!

dpgrote added 16 commits May 9, 2024 10:51

Add pec_insulator boundary condition

841770b

Give B_value a default value

cf6f799

clang-tidy fix

145e628

Add include in Source/Python/WarpX.cpp

607ba8e

Clean up const's and some spacing

60613fd

Merge branch 'development' into add_insulator_BC

611c0df

Add const requested by clang-tidy

7548cbe

Fix numeric literals

5c4cfc2

Major cleanup and reduction of the code

9c8d11a

Add documentation

e4aa5ae

Add extrapolation of tangential field

ab07778

Fix typos

ab6254b

Made ApplyPEC_InsulatortoField public

b3d9266

Fix class variable initialization

f915879

Add CI test

1e8d517

Merge branch 'development' into add_insulator_BC

82fdbe7

dpgrote added enhancement New feature or request component: boundary PML, embedded boundaries, et al. labels May 21, 2024

pre-commit-ci bot and others added 2 commits May 21, 2024 23:30

[pre-commit.ci] auto fixes from pre-commit.com hooks

8c60a03

for more information, see https://pre-commit.ci

Update documentation

a2212aa

ax3l requested a review from RemiLehe May 29, 2024 22:07

RemiLehe self-assigned this May 29, 2024

dpgrote added 8 commits June 4, 2024 11:22

Merge branch 'development' into add_insulator_BC

0846580

Merge branch 'development' into add_insulator_BC

5498195

Merge branch 'development' into add_insulator_BC

89837c6

Fix xyz_min to match other changes

1f5c72d

Merge branch 'development' into add_insulator_BC

37cbb0d

Merge branch 'development' into add_insulator_BC

16078d2

Rename CI test benchmark file

59fc8f0

Fixes related to the enum value

0973a4e

RemiLehe requested a review from EZoni September 27, 2024 20:38

RemiLehe assigned EZoni Sep 27, 2024

dpgrote added 2 commits October 7, 2024 14:12

Merge branch 'development' into add_insulator_BC

0b25492

Fixes related to MF registry

3163c2a

EZoni requested review from RevathiJambunathan and roelof-groenewald October 8, 2024 18:49

EZoni reviewed Oct 8, 2024

View reviewed changes

dpgrote and others added 3 commits October 10, 2024 11:56

Apply suggestions from code review

e074f45

Fixes in documentation Co-authored-by: Edoardo Zoni <[email protected]>

Clean up

2c3263c

Update comment

ca50303

EZoni self-requested a review October 18, 2024 21:12

EZoni changed the title ~~Add PEC_insulator bc~~ Add PECInsulator boundary condition Oct 18, 2024

Merge branch 'development' into add_insulator_BC

9a15278

dpgrote force-pushed the add_insulator_BC branch from 17fb028 to 9a15278 Compare November 4, 2024 18:31

Remove the unnecessary routine get_cell_count_to_boundary

f21e856

JustinRayAngus approved these changes Nov 5, 2024

View reviewed changes

Fix comments and documentation

5b1a429

JustinRayAngus enabled auto-merge (squash) November 5, 2024 19:54

JustinRayAngus merged commit a4d5631 into ECP-WarpX:development Nov 5, 2024
37 checks passed

dpgrote deleted the add_insulator_BC branch November 5, 2024 23:29

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add `PECInsulator` boundary condition #4943

Add `PECInsulator` boundary condition #4943

dpgrote commented May 21, 2024 •

edited by JustinRayAngus

Loading

EZoni commented Oct 8, 2024

EZoni Oct 8, 2024

EZoni Oct 8, 2024

EZoni Oct 8, 2024

dpgrote Oct 10, 2024

EZoni Oct 11, 2024

EZoni Oct 18, 2024

JustinRayAngus Nov 5, 2024

roelof-groenewald Nov 5, 2024

EZoni Oct 8, 2024 •

edited

Loading

EZoni Oct 8, 2024

dpgrote Oct 10, 2024

EZoni commented Nov 4, 2024

dpgrote commented Nov 4, 2024

dpgrote commented Nov 4, 2024

JustinRayAngus left a comment

	// Add radial scale so that drFr/dr = 0.
	// Add radial scale so that d(rF_r)/dr = 0.

	// Add radial scale so that drFr/dr = 0.
	// Add radial scale so that d(rFr)/dr = 0.

Add PECInsulator boundary condition #4943

Add PECInsulator boundary condition #4943

Conversation

dpgrote commented May 21, 2024 • edited by JustinRayAngus Loading

EZoni commented Oct 8, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

EZoni Oct 8, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

EZoni commented Nov 4, 2024

dpgrote commented Nov 4, 2024

dpgrote commented Nov 4, 2024

JustinRayAngus left a comment

Choose a reason for hiding this comment

Add `PECInsulator` boundary condition #4943

Add `PECInsulator` boundary condition #4943

dpgrote commented May 21, 2024 •

edited by JustinRayAngus

Loading

EZoni Oct 8, 2024 •

edited

Loading