Add option for runtime element distribution #5641
Conversation
knelli2
force-pushed
the
runtime_element_dist
branch
2 times, most recently
from
d9e16ad
to
f44f2de
Compare
| grid_points_per_core[target_proc] += grid_points_per_element; | ||
| grid_points_per_node[target_node] += grid_points_per_element; | ||
| if (element_weight.has_value()) { | ||
| for (const auto& element_id : element_ids) { |
There was a problem hiding this comment.
Maybe add a comment here and in the else to the effect of // Distribute by estimated cost of the element. E.g., a weighted space-filling curve. and then // Do a round-robin distribution of the elements?
| const domain::BlockZCurveProcDistribution<volume_dim> element_distribution{ | ||
| element_costs, num_of_procs_to_use, blocks, initial_refinement_levels, | ||
| initial_extents, procs_to_ignore}; | ||
| // Only need this if the element weight has a value |
There was a problem hiding this comment.
this -> element_distribution (so that when someone accidentally moves the comment or code, at least we have some hope of putting them back together :D )
Maybe expand comment ... since then we have to distribute the elements in a more complicated way than just round robin?
| element_array(element_id) | ||
| .insert(global_cache, initialization_items, target_proc); | ||
| if (element_weight.has_value()) { | ||
| const std::unordered_map<ElementId<Dim>, double> element_costs = |
There was a problem hiding this comment.
Please add same comments here that you do to the other file
| namespace OptionTags { | ||
| /// \ingroup OptionTagsGroup | ||
| /// \ingroup ComputationalDomainGroup | ||
| struct ElementDistribution { |
There was a problem hiding this comment.
Should this go into the ResourceInfo group? I can't decide whether or not I like that. I don't love having a bunch of lowest-level options if possible :Shrug:
There was a problem hiding this comment.
ResourceInfo isn't actually an options group, it's just a struct. That being said, I can put it into the struct and just add a member function for accessing it if you think that's better. The ResourceInfo is available in the initialization items, so I don't have strong opinions where we place the option
There was a problem hiding this comment.
Ah okay. Hmm, yea, if it's easy to do maybe it's worth doing? My only hesitation is then if we have an executable that uses ResourceInfo but no elements things will be confusing. In a sense we probably want something like a Parallelization group, but that seems beyond the scope of this PR and should probably have input from the other core devs 🤷 So I will let you make a decision on what you think is best and we'll just go with that :)
There was a problem hiding this comment.
Oh actually, this is true for the CCE executable. It doesn't have a DgElementArray, so with the current setup you never specify an element distribution. But if it was in the ResourceInfo, then you'd have to, but it'd be a useless option.
I like the idea of a Parallelization group. I'll put the element distribution in there, and we can add to it later with whatever else we come up with.
I guess we could technically put the ResourceInfo into Parallelization, but I think that's out of the scope of this PR.
There was a problem hiding this comment.
That sounds like a good plan to me!
knelli2
force-pushed
the
runtime_element_dist
branch
from
1508d37
to
8277396
Compare
|
@nilsdeppe Squashed. @nilsvu could you take a look at the elliptic parts that I edited? I had to change more things than expected so I'd like your sign off |
Proposed changes
Now we can choose how we distribute elements at runtime so you can experiment with what is best for your runs. Currently all the options are different weights for a space-filling curve except for a round-robin distribution. The distributions I added to each of the input files is exactly what it was before, so in practice the distribution should be the same as before.
Upgrade instructions
Evolution and elliptic executables must now have a new option in the input file called
An exception to this are the
*CharacteristicExtractexecutables. They do not need this new option.Code review checklist
make docto generate the documentation locally intoBUILD_DIR/docs/html.Then open
index.html.code review guide.
bugfixornew featureif appropriate.Further comments