Change ORB to support arbitrary decomposition axes #560
Conversation
… validation. Conserve legacy behavior.
|
cscs-ci run cscs-ci-gh200 |
|
|
|
General comment: before this can go into master we need to make sure it works with several k of GPUs. @srikrrish in ALPINE which of the tests are with ORB == ON? |
Any of the Alpine example with load balancing threshold < 1 may invoke ORB (depending on the threshold). For example the usual value of 0.01 which means load balancing should be triggered when particle load imbalance in any rank exceeds 1% is a good start. I think Penning trap with more than 8 ranks might be the good stress test. @aliemen yes the initial parallel/serial decomposition should be preserved during ORB also. If not that was a bug and if this PR fixes it that's a good thing I would say. |
| template <typename Attrib> | ||
| bool binaryRepartition(const Attrib& R, FieldLayout<Dim>& fl, | ||
| const bool& isFirstRepartition, | ||
| const std::array<bool, Dim>& allowedAxes); |
There was a problem hiding this comment.
Why do we need to have these overloads? Can't we just use this signature alone with a default argument of allowedAxes=true for all dimensions to preserve the legacy behaviour? Or is it just to avoid default arguments as it may not be a good practice (although I don't immediately see the disadvantages here)?
There was a problem hiding this comment.
I think having a default would be nicer. In 3D one could simply write std::array<bool, Dim> = {true, true, true}. But since it's dimension independent, we would either need a helper or a lambda invokation. The latter would look like this (for your second example):
int findCutAxis(
const NDIndex<Dim>& dom,
const std::array<bool, Dim>& allowedAxes = [] {
std::array<bool, Dim> axes{};
axes.fill(true);
return axes;
}());I'm honestly not sure what's cleaner. But the overload is perhaps easier to read?
If we really want a simple default argument, we could change the semantics to "forbiddenAxes". Then false would be the default (std::array<bool, Dim> allowedAxes = {}. Note that this is now by value, not by reference.
There was a problem hiding this comment.
Just let me know what you prefer, I'm fine with all three versions.
| * @param dom Domain to reduce | ||
| * @param allowedAxes true for axes ORB is allowed to cut | ||
| */ | ||
| int findCutAxis(const NDIndex<Dim>& dom, const std::array<bool, Dim>& allowedAxes); |
There was a problem hiding this comment.
Similar comment as above can't we have only one signature with a default argument?
closes #559
This PR generalizes the ORB class to allow decomposing in arbitrary axis. Now you can e.g. pass a
std::array<bool, Dim> allowedAxes = {false, false, true};array to indicate that you only want to decompose inz. The actual algorithm is more or less untouched. The addition is mostly in the helpers likefindAxis. Here an example in ourSwissFEL-booster-scinput file from the OPALX side with decomposition only inz:The decomposition works just fine only in
zdirection.Other changes:
findCutAxis(domain, allowedAxes)chooses the longest enabled axis,[1, cutAxisLength - 3],FieldLayoutand ORB weight field only after correct decomposition is found.