Overhauled and improved profiling #74
Conversation
vincepandolfo
force-pushed
the
profiling
branch
from
c718895 to
40aaa83
Compare
| if time_stepping or time_loop_stencils_b else []) | ||
| initial_block = initial_block + start_time_stmt | ||
| end_block = end_time_stmt + omp_single + ([cgen.Block(time_loop_stencils_a)] | ||
| if time_loop_stencils_a else end_time_stmt) |
There was a problem hiding this comment.
omp_single should have been inside the parenthesis here :
(omp_single + [cgen.Block(time_stepping + time_loop_stencils_b)] if time_stepping or time_loop_stencils_b else [])
to avoid empty omp sigle section breaking the compilation. Does your change fix it?
There was a problem hiding this comment.
Ok I didn't know that. I think my changes breaks it.
| return self.propagator.timings[2] | ||
|
|
||
| @property | ||
| def total_post_loops_time(self): |
There was a problem hiding this comment.
could we document pre_loops and post_loops ? what do they mean?
There was a problem hiding this comment.
pre_loop and post_loop are operations done outside of the spacial loop :
for (t=...)
pre_loop
for (x=.., y=...)
stencils, factors,...
endforxyz
post_loop
endfort
There was a problem hiding this comment.
Please confirm what I am saying but I believe pre and post loops were old names for these variables which were later changed to time_loop_stencils. During some rebase/merge conflict resolution these old names seem to have crept back in and right now both versions of these names exist in the code creating redundancy. AFAIK, these old names pre_loop and post_loop are not being used anywhere in the code and I have been meaning to open a PR to remove them (again).
If you can confirm the above, please feel free to remove this.
There was a problem hiding this comment.
To be honest I have not seen those names anywhere in the code. I used them just because they were much shorter than time_loops_stencils.
Right now I am working on completely redoing the profiler by making a profiler class. I will close this PR until the new version is ready (it should be ready tomorrow) if it's not urgent to have this one merged.
|
This needs to be tested on MIC. |
|
Works on MIC |
|
How did you test this? On Mon, 8 Aug 2016 at 15:46 Vincenzo Pandolfo [email protected]
|
|
@pvelesko did |
The aim of this PR is to improve the profiling capabilities of Devito (also fixing issue #65 )
Propagator.cfunctionhas been moved down to the propagator. This should also make easier to execute it when the user uses the low levelPropagatorinterface directly.