Bisection search for split_into_three #976
Conversation
|
As a demonstration of the performance enhancement, the time for |
| int best_low_split_point; | ||
| direction best_low_split_direction; | ||
| double left_effort_fraction; | ||
| find_best_split(3, best_low_split_point, best_low_split_direction, left_effort_fraction); |
There was a problem hiding this comment.
I don't think the find_best_split method is really correct right now for desired_chunks != 2. That's why @ChristopherHogan had a separate split_into_three function in the first place?
There was a problem hiding this comment.
At first glance, I think we should change two places:
-
cost_diffshould returnright_cost - left_cost * (desired_chunks-1); -
find_best_splitshould setsplit_measuretomax(left_cost * (desired_chunks-1), right_cost)
There was a problem hiding this comment.
For example, if desired_chunks == 3 then you would ideally like the left_cost to be 1/3 of the total and right_cost to be 2/3 of the total, so basically you want right_cost == (desired_chunks-1) * left_cost
There was a problem hiding this comment.
For the specific cases of desired_chunks == 1 or == 2, my proposal is equivalent to what it is doing now. But I think it would be better to change it to my formula, which seems more general.
* Bisection search for split_into_three * update split_measure * whoops * whoops again
Extends the bisection search approach of #966 to
grid_volume::split_into_threewhich was missing.