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

Implicit add filtering #5086

Merged

Conversation

dpgrote
Copy link
Member

@dpgrote dpgrote commented Jul 26, 2024

This adds filtering to the implicit solver, replacing PR #4600.

It is a simple change. All that is needed is adding a call to filter the Efield_fp just before the particles are pushed. The current density is already filtered in SyncCurrentAndRho.

The name of the routine ApplyFilterJ was changed to ApplyFilterMF since it now has a more general usage.

@dpgrote dpgrote added the component: implicit solvers Anything related to implicit solvers label Jul 26, 2024
@dpgrote dpgrote requested a review from JustinRayAngus July 26, 2024 21:44
Comment on lines 572 to 573
if (use_filter) { ApplyFilterJ(current_fp_vay, lev); }
if (use_filter) { ApplyFilterMF(current_fp_vay, lev); }
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Should ApplyFilterJ() be different than applying a filter for E or B when using curvilinear geometries?

I believe the filter should be applied to the current before dividing by the volume element to convert to current density. Perhaps this is being done somewhere else?

Copy link
Member Author

Choose a reason for hiding this comment

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

You are probably correct, but filtering doesn't work for RZ as it is now (at least for FDTD). As it is now, the inverse volume scaling is done before the filtering. That order could be changed but that would need to be a separate PR.

@RemiLehe RemiLehe self-assigned this Jul 29, 2024
@JustinRayAngus
Copy link
Contributor

JustinRayAngus commented Jul 31, 2024

This looks good to me.

We could also add filtering to the magnetic field as an option. That should probably be a separate PR though.

Copy link
Contributor

@JustinRayAngus JustinRayAngus left a comment

Choose a reason for hiding this comment

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

Thanks Dave for this PR!

@JustinRayAngus JustinRayAngus merged commit 9476692 into ECP-WarpX:development Nov 11, 2024
37 checks passed
@dpgrote dpgrote deleted the implicit_add_filtering_redo branch November 11, 2024 17:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: implicit solvers Anything related to implicit solvers
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants