Skip to content

[Particle] Improve bin weight estimate - #2145

Open
vovannikov wants to merge 3 commits into
4C-multiphysics:mainfrom
vovannikov:particle_engine_improve_weights
Open

[Particle] Improve bin weight estimate#2145
vovannikov wants to merge 3 commits into
4C-multiphysics:mainfrom
vovannikov:particle_engine_improve_weights

Conversation

@vovannikov

Copy link
Copy Markdown
Contributor

Description and Context

This feature evaluates weights for particle bins based on the number of interaction pairs a bin contains. This information is already available, since we construct neighbors anyways. As the outcome, it improves the particle distribution and optimizes the load balance, especially when the work load per rank is rather large, e.g. >300,000 particles/rank. For smaller number of particles per rank, e.g. ~50,000, the effect is less significant but still present.

image

Related Issues and Pull Requests

#2117, #2074

// build particle-to-particle neighbors only, to populate bin_interaction_costs_ for
// cost-aware rebalancing below; the global id map and wall neighbor relations are skipped
// here since both would be invalidated by the redistribution pass right after anyway
build_potential_neighbor_relation(/*include_wall_neighbors=*/false);

Copy link
Copy Markdown
Member

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_element calls in interaction_sph_neighbor_pairs and interaction_dem_neighbor_pairs.

Copy link
Copy Markdown
Contributor Author

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.

Copy link
Copy Markdown
Contributor

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.

Copy link
Copy Markdown
Contributor Author

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.


// 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] +=

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The 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 build_particle_to_particle_neighbors() should only build pontential particle neighbors.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The 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 bin_interaction_costs_ is not something that is absolutely independent, on the contrary, now the coupling in the code is quite tight, which was the exact intent. However, I don't have a strong opinion on this.

What bothers me more - is actually the overhead when moving the population of bin_interaction_costs_ into a different function. I could have just iterated over potentialparticleneighbors_ as it contains the neighbors types (still, there is overhead here but not significant) but there is no gidofbin info available. Unless there is a quick way to get it from a potentialparticleneighbors_ entry. Is something like this available?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The 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.

@jeremylt

jeremylt commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

Two questions here

  1. Do you have a sense or data on what this does for per-process heterogeneity? Roughly speaking, we'll get better GPU performance with better per-process particle heterogeneity, as long as we have enough particles per process.

  2. Do you have the configurations for these tests somewhere I can use? I'd like to make sure my GPU porting efforts aren't impacting CPU-only runs negatively.

@vovannikov

Copy link
Copy Markdown
Contributor Author

Two questions here

  1. Do you have a sense or data on what this does for per-process heterogeneity? Roughly speaking, we'll get better GPU performance with better per-process particle heterogeneity, as long as we have enough particles per process.
  2. Do you have the configurations for these tests somewhere I can use? I'd like to make sure my GPU porting efforts aren't impacting CPU-only runs negatively.

@jeremylt

  1. This is a very good question. I have not really checked the costs of different particle types in details. What this algorithm does, it can indeed put a bit more particles on one rank and a fewer number on another based on how expensive the operations with the particles are. But this strategy can (and probably should) be changed together with the binning strategy.
  2. I have sent you the benchmark and the scripts for running in it and postprocessing the measurements data.

@vovannikov

Copy link
Copy Markdown
Contributor Author

What do you guys think about this feature?

I have a feeling, that given the ongoing GPU transition, most probably we will need to reconsider the particle redistribution algorithm grouping the particles according to their types. In this context, this feature can serve as a good benchmark reference to measure the obtained imbalance for new approaches.

@jeremylt

Copy link
Copy Markdown
Contributor

This looks like a clear win for CPU workloads, and we can always adjust the binning strategy when running for the GPU if we determine it is a bottleneck.

@slfuchs

slfuchs commented Aug 11, 2026

Copy link
Copy Markdown
Contributor

I have the same opinion than @jeremylt , so let's proceed here and get it into the master. Later on we can still refine or adapt if necessary.

slfuchs
slfuchs previously approved these changes Aug 11, 2026
georghammerl
georghammerl previously approved these changes Aug 11, 2026
@vovannikov
vovannikov dismissed stale reviews from georghammerl and slfuchs via ed63cd3 August 12, 2026 08:38
@vovannikov
vovannikov force-pushed the particle_engine_improve_weights branch from 6599910 to ed63cd3 Compare August 12, 2026 08:38
@jeremylt

Copy link
Copy Markdown
Contributor

A note on the casting, we could do EnumTools::enum_integer(foo) instead of static_cast<int>(foo) if that's preferred, but it wouldn't save us any space so its whatever seems clearer to adopt as the general preference

@ppraegla

Copy link
Copy Markdown
Member

A note on the casting, we could do EnumTools::enum_integer(foo) instead of static_cast<int>(foo) if that's preferred, but it wouldn't save us any space so its whatever seems clearer to adopt as the general preference

Good point. I suggest sticking to static_cast<int>. To me it seems to be the standard way to convert enum classes. And we do not rely on an external library for this simple task. magic_enum is better suited for obtaining the string representation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants