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

Fixing the computation of ECT Rho Field #2711

Merged
merged 13 commits into from
Feb 3, 2022

Conversation

lgiacome
Copy link
Member

@lgiacome lgiacome commented Dec 27, 2021

This PR introduces a few fixes to the ECT solver.
Initially the 3D test in Examples/Tests/PEC failed when using the ECT solver instead of Yee.
This is the 1D plot of the field we would obtain with ECT:
Ey_pec_analysis_ECT_old
While this is what we would have with Yee:
Ey_pec_analysis_PEC

There were two issues:

  1. a problem with the initialisation of the fields would produced the spurious field at the left
  2. the magnitude of the standing wave was lower in the ECT case

In both cases the problem is that the ECTRho field was not recomputed correctly after the electric field was modified. In the second case the E field is modified by the initialisation and in the second case the E field is modified by the boundary conditions.

With this PR we add the missing calls to EvolveRhoCartesianECT and we introduce the new function EvolveECTRho for consistency with the update of the other fields and in view of possible extensions of the ECT algorithm (e.g. RZ).

With these changes the test passes also with the ECT algorithm and we retrieve the same results as with the Yee solver.

@lgiacome lgiacome requested a review from RemiLehe December 27, 2021 12:54
@lgiacome lgiacome added component: boundary PML, embedded boundaries, et al. bug: affects latest release Bug also exists in latest release version labels Dec 27, 2021
@ax3l ax3l mentioned this pull request Feb 2, 2022
@ax3l ax3l added the bug Something isn't working label Feb 2, 2022
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.

Looks good to me. Thanks a lot for this PR!
I have a quick question (see inline comment)

}

#ifndef WARPX_DIM_RZ
void FiniteDifferenceSolver::EvolveRhoCartesianECT (
Copy link
Member

Choose a reason for hiding this comment

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

The fact that this function was copied to a different file makes it difficult to see whether were any changes.
@lgiacome Could you confirm that there was no change in this function?

Copy link
Member Author

Choose a reason for hiding this comment

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

Correct, there were no changes. It was just a copy-paste :)

Copy link
Member

@EZoni EZoni left a comment

Choose a reason for hiding this comment

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

Thank you for this PR, Lorenzo! I left a few questions and comments.

lgiacome and others added 7 commits February 3, 2022 01:59
Co-authored-by: Axel Huebl <axel.huebl@plasma.ninja>
Co-authored-by: Edoardo Zoni <59625522+EZoni@users.noreply.github.com>
Co-authored-by: Edoardo Zoni <59625522+EZoni@users.noreply.github.com>
Copy link
Member

@EZoni EZoni left a comment

Choose a reason for hiding this comment

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

Thank you for making the changes! Looks good to me.

Co-authored-by: Axel Huebl <axel.huebl@plasma.ninja>
@ax3l ax3l enabled auto-merge (squash) February 3, 2022 01:35
@ax3l ax3l removed the request for review from RevathiJambunathan February 3, 2022 01:36
@ax3l ax3l merged commit c3c88df into ECP-WarpX:development Feb 3, 2022
@lgiacome lgiacome deleted the fix_ect_rho branch February 3, 2022 09:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug: affects latest release Bug also exists in latest release version bug Something isn't working component: boundary PML, embedded boundaries, et al.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants