[WIP] Add regions and BLProf only macros. #3270
Conversation
kngott
changed the title
|
This is associated with the AMReX PR: AMReX-Codes/amrex#2891 So, including that in the build would also show particle comm data. |
ax3l
added
the
component: parallelization
| @@ -80,6 +80,7 @@ WarpX::Evolve (int numsteps) | |||
| const int step_begin = istep[0]; | |||
| for (int step = istep[0]; step < numsteps_max && cur_time < stop_time; ++step) | |||
| { | |||
| WARPX_PROFILE_REGION_START("WarpX::Evolve::step"); | |||
There was a problem hiding this comment.
Wait is WARPX_PROFILE not sufficient here?
There was a problem hiding this comment.
I specifically added and used PROFILE_REGION_START and REGION_STOP because START and STOP do not add a region to TinyProfiler outputs, only Full Profiler outputs. So, this change would not impact the output of most users runs. (Too many regions can make parsing the output impossible.)
They can be easily changed to a PROFILE_REGION (see line 64 for the only REGION example so far) if you also want an isolated TInyProfiler region out for each unique REGION you instrument.
| @@ -394,7 +396,9 @@ WarpX::OneStep_nosub (Real cur_time) | |||
| ExecutePythonCallback("particlescraper"); | |||
| ExecutePythonCallback("beforedeposition"); | |||
|
|
|||
| WARPX_PROFILE_REGION_START("WarpX::nosub::PushParticles"); | |||
| PushParticlesandDepose(cur_time); | |||
There was a problem hiding this comment.
Do we directly want to put a WARPX_PROFILE inside PushParticlesandDepose instead?
There was a problem hiding this comment.
It could. Up for discussion. I stuck to wrapped section of Evolve and OneStep for now, as you generally want bigger, all-encompassing blocks of code for the FullProfiler analysis and probably matching these regions with the overall flow of the code rather than specific functions seemed like a better deal.
(After reading through the code, I was picturing all the OneStep functions instrumented in a matching way, meaning the profile gives consistent cuts no matter how you run WarpX.)
| @@ -403,11 +407,13 @@ WarpX::OneStep_nosub (Real cur_time) | |||
| // the actual current J. This is computed later in WarpX::PushPSATD, by calling | |||
| // WarpX::PSATDVayDeposition. The function SyncCurrent is called after that, | |||
| // instead of here, so that we synchronize the correct current. | |||
| WARPX_PROFILE_REGION_START("WarpX::nosub::SyncJ_rho"); | |||
There was a problem hiding this comment.
Directly in SyncCurrent and SyncRho as WARPX_PROFILE?
There was a problem hiding this comment.
It could. Same as above.
(Apparently, while switching focus between windows, I clicked right on the "resolve" button, so don't take that as any commentary. 😄)
First draft at adding profiling regions to the code.
Adds the Full Profiler only-macros, so this pass doesn't affect the current output, and took my best guess at reasonable regions for the UniformPlasma and KPP tests. First runs seem to work.
Iteration and discussion of where to put regions, and Full Profiler or TinyProfiler regions are welcome.