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

New embedded boundary data structures in preparation for #5534 #5574

Merged
merged 34 commits into from
Jan 21, 2025

Conversation

RemiLehe
Copy link
Member

@RemiLehe RemiLehe commented Jan 17, 2025

When 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.

@roelof-groenewald
Copy link
Member

Thanks for going through the effort to run this test!

It is interesting that the failing test test_3d_embedded_boundary_rotated_cube
asks for the same updated checksum values in this PR than in the one with the staircases moved:
This CI run:

New checksums file test_3d_embedded_boundary_rotated_cube.json:
{
  "lev=0": {
    "Bx": 1.252616939910365e-05,
    "By": 0.02473895628331097,
    "Bz": 0.024738956316621142,
    "Ex": 10253221.850298548,
    "Ey": 10387.334582977643,
    "Ez": 10387.532806510022
  }
}

The updated checksum values in the PR with the moved staircase:

{
  "lev=0": {
    "Bx": 1.252616939910365e-05,
    "By": 0.02473895628331097,
    "Bz": 0.024738956316621142,
    "Ex": 10253221.850298548,
    "Ey": 10387.334582977643,
    "Ez": 10387.532806510022
  }
}

@RemiLehe RemiLehe force-pushed the test_previous_checksum branch from 72d2e26 to a6eba80 Compare January 18, 2025 16:39
@RemiLehe RemiLehe force-pushed the test_previous_checksum branch from a6eba80 to 4b4e761 Compare January 18, 2025 18:20
@RemiLehe
Copy link
Member Author

@roelof-groenewald Yes, I think I found the reason for this and fixed it in this commit:
ace399b
(which also been added to the original PR #5534)

The reason is that, with the ECT solver, MarkExtensionCells()/ ComputeFaceExtensions modify the face_areas, so MarkUpdateBCellsECT (which depends on face_areas) should be called after MarkExtensionCells()/ ComputeFaceExtensions.

@roelof-groenewald
Copy link
Member

roelof-groenewald commented Jan 19, 2025

Cool! It is awesome that all the CI tests pass now without any checksum value changes. Seeing as you went through the work for this PR, we could merge this one as a purely code-cleaning PR, and then have #5534 be the PR that specifically changes the way the staircase-approximation is done. What do you think?
Doing it that way would provide a nice code branching point in case anybody in the future wants to test things with the old staircase setup.

@roelof-groenewald roelof-groenewald added the cleaning Clean code, improve readability label Jan 19, 2025
@RemiLehe RemiLehe changed the title [Do not merge] Test checksums with new EB structures New embedded boundary data structures in preparation for #5534 Jan 21, 2025
@RemiLehe RemiLehe merged commit e3378b0 into ECP-WarpX:development Jan 21, 2025
29 of 31 checks passed
roelof-groenewald pushed a commit that referenced this pull request Jan 21, 2025
# Overview

This PR changes the definition of the stair-case approximation of the EB
(used in the Yee solver), so that the actual EB boundary (where e.g.
particles are removed from the simulation) is **inside** the
stair-case-approximated boundary - as opposed to the definition used in
the current `development` branch, for which the actual EB is **outside**
the stair-case-approximated boundary.

This ensures that the algorithm remains charge conserving, when charged
particles are absorbed or emitted by the embedded boundary, **for
particle shape of order 1**. (Higher-order particle shapes will be
addressed in #5209.) As illustrated in the figure below (and as
discussed in [this
paper](https://www.researchgate.net/publication/318642364_Charge_Conserving_Emission_from_Conformal_Boundaries_in_Electromagnetic_PIC_simulations)),
this is fundamentally because the particle does not deposit any charge
in the valid cells, at the time when it is removed/emitted.

<img width="608" alt="Screenshot 2025-01-12 at 8 56 17 AM"
src="https://github.com/user-attachments/assets/bd803ef0-faf8-4ea5-973b-240c15b2ba4b"
/>

(The black crosses show the locations where the electric field is not
updated, and thus usually remains equal to 0. The red dots show the
locations where the particle deposits charge, for particle shape of
order 1.)

The better behavior with respect to charge-conservation can be observed
in the following animations, which show two particles of opposing charge
separating and going into the embedded boundary. (Note that a static
error in `divE` remains at the position where the particle was absorbed,
with the `development` branch. The propagating errors in `divE` are
expected: they are due to electromagnetic waves reflecting on the EB.)

- **development branch**

![movie](https://github.com/user-attachments/assets/d486663d-a182-4751-b1d1-709b1a74ea44)

- **this PR**

![movie](https://github.com/user-attachments/assets/94a5dea3-2bb8-4548-b320-7615cac86fe7)

Input script:
[inputs.txt](https://github.com/user-attachments/files/18428873/inputs.txt)
Analysis script:
[openPMD-visualization.ipynb.txt](https://github.com/user-attachments/files/18428878/openPMD-visualization.ipynb.txt)

(An automated tests using a similar configuration has been added in a
separate, follow-up PR: #5562)

Note that, as part of this PR, the above new definition has been adopted
for all the finite-difference solvers, except for the ECT solver (which
uses a cut-cell representation instead of a stair-case representation).

# Implementation

This PR uses the changes of #5574. It still uses `MarkUpdateECellsECT`
and `MarkUpdateBCellsECT` for the ECT sover - which preserve the
previous behavior of the embedded boundary for this solver, but now uses
`MarkUpdateCellsStairCase` for the other FDTD solvers - which introduce
the above-mentioned new stair-case definition.
RemiLehe added a commit that referenced this pull request Jan 22, 2025
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.

---------

Co-authored-by: Roelof Groenewald <[email protected]>
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
@ax3l ax3l added the component: boundary PML, embedded boundaries, et al. label Jan 28, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cleaning Clean code, improve readability component: boundary PML, embedded boundaries, et al.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants