SpaceTimeStack: Account for MPI imbalance when applying printing threshold

[vbrunini]

So that entries with a low average time but high imbalance are reported since they are often the cause of significant performance problems due to load imbalance.

Also fix missing deep_copy hook exposure.

[vlkale]

…shold.

So that entries with a low average time but high imbalance are reported since they are often the cause of significant performance problems due to load imbalance.

Also fix missing deep_copy hook exposure.

@vbrunini

Thanks for this. I am curious if you can provide an example code with the problem here in the PR description. Even better would be a set of tests via, say, Google Test, to check that those entries with low average time but high imbalance are indeed being reported.

A side note is that the fix for the missing deep_copy hook you have should ideally be a separate PR, as it looks conceptually unrelated to the change related to MPI load imbalance?

Thanks!

[vbrunini]

…shold.
So that entries with a low average time but high imbalance are reported since they are often the cause of significant performance problems due to load imbalance.
Also fix missing deep_copy hook exposure.

@vbrunini

Thanks for this. I am curious if you can provide an example code with the problem here in the PR description. Even better would be a set of tests via, say, Google Test, to check that those entries with low average time but high imbalance are indeed being reported.

A side note is that the fix for the missing deep_copy hook you have should ideally be a separate PR, as it looks conceptually unrelated to the change related to MPI load imbalance?

Thanks!

Added a test (and split off the deep copy parts into #285)

[masterleinad]

Can you elaborate on why this is sensible? I understand the previous code but now we essentially do

((max_runtime / avg_runtime) * percent  < outout_threshold

In other words, why does multiplying by max_runtime / avg_runtime do what you want? Wouldn't you want to do something like

if (percent > first_threshold || (high_imbalance && percent > second_threshold))
  report

? It's not quite clear to me that this second threshold should be the same as the orignal one divided by the imbalance. Maybe it's sensible to report all kernels that have an imbalance greater than a certain factor?

[vbrunini]

The thinking is to identify any kernels/regions that would be above the 0.1% threshold if we only looked at the runtimes on that rank. Assuming that total_runtime is evenly distributed across ranks (which I think is basically always true), then rank_wall_time = tree_time / comm_size and therefore max_runtime / rank_wall_time == (max_runtime * comm_size) / tree_time.

[masterleinad]

OK. You basically want to use max_runtime instead of avg_runtime for comparing with the threshold value. I think that's sensible. Not sure if we need a configuration option.
Note that there also is print_json_recursive that needs consistent logic.

[masterleinad]

You might as well compute this properly once and store it in a member variable.

[vbrunini]

Any suggestions for what to do about the CI failure from the tests running as root?

--------------------------------------------------------------------------
mpirun has detected an attempt to run as root.

Running as root is *strongly* discouraged as any mistake (e.g., in
defining TMPDIR) or bug can result in catastrophic damage to the OS
file system, leaving your system in an unusable state.

We strongly suggest that you run mpirun as a non-root user.

You can override this protection by adding the --allow-run-as-root option
to the cmd line or by setting two environment variables in the following way:
the variable OMPI_ALLOW_RUN_AS_ROOT=1 to indicate the desire to override this
protection, and OMPI_ALLOW_RUN_AS_ROOT_CONFIRM=1 to confirm the choice and
add one more layer of certainty that you want to do so.
We reiterate our advice against doing so - please proceed at your own risk.
--------------------------------------------------------------------------