-
Notifications
You must be signed in to change notification settings - Fork 68
[Particle] Improve bin weight estimate #2145
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -468,8 +468,9 @@ void Particle::ParticleEngine::build_particle_to_particle_neighbors() | |
| // relate half neighboring bins to owned bins | ||
| if (not binning_->validhalfneighboringbins_) relate_half_neighboring_bins_to_owned_bins(); | ||
|
|
||
| // clear potential particle neighbors | ||
| // clear potential particle neighbors and accumulated interaction costs | ||
| potentialparticleneighbors_.clear(); | ||
| bin_interaction_costs_.clear(); | ||
|
|
||
| // invalidate flag denoting validity of particle neighbors map | ||
| validparticleneighbors_ = false; | ||
|
|
@@ -554,6 +555,11 @@ void Particle::ParticleEngine::build_particle_to_particle_neighbors() | |
| potentialparticleneighbors_.push_back( | ||
| std::make_pair(std::make_tuple(type, Owned, ownedindex), | ||
| std::make_tuple(neighbortype, neighborstatus, neighborindex))); | ||
|
|
||
| // accumulate interaction cost for load balancing weight estimation, weighted by the | ||
| // average of the dynamic load balance factors of the two interacting particle types | ||
| bin_interaction_costs_[gidofbin] += | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I would prefer to move this to a separate function. I like to keep the concept, that one method is doing only one thing. So
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The potential particle neighbors is exactly the data this criterium relies on. So in that sense What bothers me more - is actually the overhead when moving the population of
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. you are right, since the global id of the bin is needed it probably makes sense to leave it as it is. |
||
| 0.5 * (typeweights_[type] + typeweights_[neighbortype]); | ||
| } | ||
| } | ||
| } | ||
|
|
@@ -2064,12 +2070,19 @@ void Particle::ParticleEngine::determine_bin_weights() | |
| // get global id of bin | ||
| const int gidofbin = binning_->binrowmap_->gid(rowlidofbin); | ||
|
|
||
| // iterate over owned particles in current bin | ||
| for (const auto& particleIt : particlestobins_[binning_->bincolmap_->lid(gidofbin)]) | ||
| // use neighbor pair count from the previous interaction evaluation as a proxy for the | ||
| // compute cost of this bin; fall back to particle count * type weight when no interaction | ||
| // data is available yet (e.g. on the very first load redistribution) | ||
| auto costIt = bin_interaction_costs_.find(gidofbin); | ||
| if (costIt != bin_interaction_costs_.end()) | ||
| { | ||
| binning_->binweights_->get_vector(0).get_values()[rowlidofbin] += costIt->second; | ||
| } | ||
| else | ||
| { | ||
| // add weight of particle of specific type | ||
| binning_->binweights_->get_vector(0).get_values()[rowlidofbin] += | ||
| typeweights_[particleIt.first]; | ||
| for (const auto& particleIt : particlestobins_[binning_->bincolmap_->lid(gidofbin)]) | ||
| binning_->binweights_->get_vector(0).get_values()[rowlidofbin] += | ||
| typeweights_[particleIt.first]; | ||
| } | ||
| } | ||
| } | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder if the assumption of only the particle-particle interaction entering the cost function is fair? The evluation particle-wall is (what I would guess) way more expensive as the nonlinear contact point search needs to be performed (see
Core::Geo::nearest_3d_object_on_elementcalls ininteraction_sph_neighbor_pairsandinteraction_dem_neighbor_pairs.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That is a very good point! I thought of this. But this issue existed even before this modification. Strictly speaking, that's why we have customizable weights per particle type. Here the weights are averaged within each contact, so if a user provided custom weights, then this information is also accounted for. But it would be nice to have this data provided in more automatic manner rather then user provided weights, this will then further resolve the imbalance issue.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The reason why I neglected particle-wall pairs respectively wall element counts for the bin distribution is, that in most cases the wall is surrounding the particle domain and coupling is performed only on the interface. So my aim was to optimize for ideal distribution of particles among procs rather than wall elements among procs. Note that the structural elements in the structural solver are distributed based on graph partitioning. That said, and also for the sake of ideally cubic processor domains, only the particles are considered for the bin distribution. I think, that considering particle-wall pairs in the bin distribution might lead to more lengthy shaped processor domains, such that each processor gets a share of wall elements and consequently this leads to more communication between the processors due to the worse ratio of processor domain boundary to volume.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@slfuchs that makes sense. It would be actually interesting to measure the effect of particle-wall interactions on the load imbalance. The benchmark I have been using so far has no walls. I guess, the best way is to run the same geometry with the rigid boundary particles and the particle walls and measure the load imbalance. Anyways, this is one of those things that, in my opinion, needs profiling and measuring. And I would postpone it for a different PR.