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

WarpX class: move shiftMF to anonymous namespace in WarpXMovingWindow.cpp #5609

Open
wants to merge 6 commits into
base: development
Choose a base branch
from

Conversation

lucafedeli88
Copy link
Member

@lucafedeli88 lucafedeli88 commented Jan 27, 2025

This PR moves the static function shiftMF from the WarpX class to an anonymous namespace in WarpXMovingWindow.cpp, where it is actually used.
This is done to simplify the Warpx class.

@lucafedeli88 lucafedeli88 added the cleaning Clean code, improve readability label Jan 27, 2025
@lucafedeli88 lucafedeli88 changed the title [WIP] WarpX class: move shiftMF to anonymous namespace in WarpXMovingWindow.cpp WarpX class: move shiftMF to anonymous namespace in WarpXMovingWindow.cpp Jan 29, 2025
const bool PMLRZ_flag = false)
{
using namespace amrex::literals;
WARPX_PROFILE("::shiftMF()");
Copy link
Member

Choose a reason for hiding this comment

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

Does this still have a sensible prefix in the BL Profiler?

Copy link
Member Author

Choose a reason for hiding this comment

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

It depends on what you mean for sensible ;-) ! It appears like that at the moment:

---------------------------------------------------------------------------------------------------
Name                                                NCalls  Excl. Min  Excl. Avg  Excl. Max   Max %
---------------------------------------------------------------------------------------------------
::shiftMF()                                            591     0.8066     0.8066     0.8066  18.74%
WarpX::OneStep_nosub()                                 100     0.6293     0.6293     0.6293  14.62%
PhysicalParticleContainer::Evolve()                    100     0.4915     0.4915     0.4915  11.42%
ParticleContainer::RedistributeCPU()                   304     0.4015     0.4015     0.4015   9.33%
FillBoundary_nowait()                                 1819     0.2607     0.2607     0.2607   6.06%
Filter::ApplyStencil(MultiFab)                         303     0.2414     0.2414     0.2414   5.61%
FlushFormatOpenPMD::WriteToFile()                        2     0.2344     0.2344     0.2344   5.45%

Do you have an alternative in mind? Just shiftMF maybe ?

@@ -57,6 +57,184 @@

using namespace amrex;

namespace
{
void shiftMF (
Copy link
Member

Choose a reason for hiding this comment

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

Adding a Doxygen string on the way would be really nice :)

Copy link
Member Author

Choose a reason for hiding this comment

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

I agree!

Copy link
Member Author

Choose a reason for hiding this comment

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

Done!

const int nc = mf.nComp();
const amrex::IntVect& ng = mf.nGrowVect();

AMREX_ALWAYS_ASSERT(ng[dir] >= std::abs(num_shift));
Copy link
Member Author

Choose a reason for hiding this comment

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

Previously, this was AMREX_ALWAYS_ASSERT(ng[dir] >= num_shift);
however, it seems to me that, since num_shift can be negative, we need an absolute value here. What do you think @ax3l ?

} else {
amrex::IntVect ng_mw = amrex::IntVect::TheUnitVector();
// Enough guard cells in the MW direction
ng_mw[dir] = std::abs(num_shift);
Copy link
Member Author

Choose a reason for hiding this comment

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

Same as above. This was ng_mw[dir] = num_shift;, but I think that the absolute value is necessary here.

@lucafedeli88 lucafedeli88 requested a review from ax3l January 30, 2025 15:34
@ax3l ax3l self-assigned this Feb 4, 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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants