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

Do Not Fill Guard Cells with Inverse FFTs, Unless for Field Damping #2045

Merged
merged 10 commits into from
Jul 8, 2021

Conversation

EZoni
Copy link
Member

@EZoni EZoni commented Jul 1, 2021

Fixes #2035.

Description
I added a new static member to the WarpX class, the IntVect fill_guards. It is an AMREX_SPACEDIM-dimensional vector of integers (0 or 1 should be the only allowed values), to control whether to fill the guard cells when performing inverse FFTs, independently along each direction. When fill_guards = 0 in some direction, the full box used for the loop in the inverse FFT is shrinked by the number of guard cells in that direction.

WarpX::fill_guards is currently set to 0 in each direction, unless the direction is non-periodic and field damping is applied (e.g. with a moving window).

@EZoni EZoni added bug Something isn't working component: spectral Spectral solvers (PSATD, IGF) bug: affects latest release Bug also exists in latest release version labels Jul 1, 2021
@EZoni EZoni force-pushed the bugfix_iFFT_ghost branch 3 times, most recently from 3a14f40 to a9799c5 Compare July 1, 2021 23:16
@EZoni EZoni force-pushed the bugfix_iFFT_ghost branch 3 times, most recently from 0b014e8 to 2719831 Compare July 2, 2021 00:07
@EZoni EZoni changed the title [WIP] Do Not Always Fill Guard Cells with Inverse FFTs Do Not Always Fill Guard Cells with Inverse FFTs Jul 2, 2021
@EZoni EZoni marked this pull request as ready for review July 2, 2021 05:52
@EZoni EZoni changed the title Do Not Always Fill Guard Cells with Inverse FFTs Fill Guard Cells with Inverse FFTs? New Input Parameter Jul 2, 2021
@EZoni EZoni changed the title Fill Guard Cells with Inverse FFTs? New Input Parameter [WIP] Fill Guard Cells with Inverse FFTs? New Input Parameter Jul 6, 2021
@EZoni EZoni force-pushed the bugfix_iFFT_ghost branch from b534bf6 to 5421465 Compare July 6, 2021 22:59
@EZoni
Copy link
Member Author

EZoni commented Jul 6, 2021

@RemiLehe I suggest to have a look here before I update the benchmarks, because the failing tests on Azure will print out the relative errors of each failing benchmark. This will help us get an idea of which tests fail and how much they fail. So I will commit the update of the benchmarks after all CI checks have finished, following the last commit I just pushed (b85faab).

@EZoni EZoni changed the title [WIP] Fill Guard Cells with Inverse FFTs? New Input Parameter [WIP] Do Not Fill Guard Cells with Inverse FFTs, Unless for Field Damping Jul 7, 2021
@EZoni EZoni changed the title [WIP] Do Not Fill Guard Cells with Inverse FFTs, Unless for Field Damping Do Not Fill Guard Cells with Inverse FFTs, Unless for Field Damping Jul 7, 2021
@EZoni EZoni requested a review from RemiLehe July 7, 2021 22:44
@EZoni EZoni requested review from ax3l and NeilZaim July 7, 2021 22:44
Copy link
Member

@ax3l ax3l left a comment

Choose a reason for hiding this comment

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

Following our discussions and looking at the code, the current solutions looks good to me.

I'll let @RemiLehe take the final look.

Copy link
Member

@NeilZaim NeilZaim left a comment

Choose a reason for hiding this comment

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

That looks good to me, thanks!

If we wanted to be more general, we could consider having a fill_guards_lo and a fill_guards_hi but I don't know if there are situations where that would be useful. (e.g., if there's a scenario where field_boundary_lo[dir] is Damped and field_boundary_hi[dir] isn't)

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, thanks!

@RemiLehe RemiLehe merged commit 6269c20 into ECP-WarpX:development Jul 8, 2021
@EZoni EZoni deleted the bugfix_iFFT_ghost branch July 8, 2021 14:57
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: spectral Spectral solvers (PSATD, IGF)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Instability when Filling Guard Cells with Inverse FFTs
4 participants