[beaminteraction] Avoid binning based partitioning #1523
Conversation
maxfirmbach
added
the
type: enhancement
davidrudlstorfer
left a comment
davidrudlstorfer
left a comment
There was a problem hiding this comment.
One question - in #180 we decided that we leave both options (old and new one) working to allow for an easy comparison between the new and old method. Is this now possible with the current state? Or only for "cross-linking simulations"? Sorry for the dumb question 😅
| if (beaminteraction_params_ptr_->get_search_strategy() == | ||
| Inpar::BeamInteraction::SearchStrategy::bounding_volume_hierarchy) | ||
| { | ||
| const auto geometric_search_params_ptr_ = Core::GeometricSearch::GeometricSearchParams( | ||
| Global::Problem::instance()->geometric_search_params(), | ||
| Global::Problem::instance()->io_params()); |
There was a problem hiding this comment.
Would it make sense to separate this into two different functions depending on the search strategy to be better separated?
There was a problem hiding this comment.
I agree, that also makes reviewing a lot easier.
There was a problem hiding this comment.
@davidrudlstorfer, do you mean one if up front? Yes, I'd prefer that, so that one has to make the decision only once and not cluttering the code with multiple instances of the same if.
|
Thanks for the changes @maxfirmbach . Can we switch the default to the new partitioning method and see which tests pass? Then I think we are actually not really far from merging this. |
4 participants
Description and Context
This PR aims to avoid the binning based partitioning in the beaminteraction framework and proposes the option of building a monolithic graph and using graph partitioning instead.
Put as draft for now for further discussion.