Skip to content
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.

Sign up for GitHub

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

Bisection search for split_into_three #976

Merged
merged 4 commits into from
Aug 7, 2019
Merged

Bisection search for split_into_three #976

Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
240388e
Bisection search for split_into_three
oskooi Jul 29, 2019
d1a2e7e
update split_measure
oskooi Aug 1, 2019
f808e05
whoops
oskooi Aug 1, 2019
1e0452e
whoops again
oskooi Aug 1, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
94 changes: 23 additions & 71 deletions src/vec.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1127,77 +1127,29 @@ grid_volume grid_volume::split_by_cost(int desired_chunks, int proc_num, int bes
}

void grid_volume::split_into_three(std::vector<grid_volume> &result) const {

grid_volume best_low, best_mid, best_high;
double best_overall_split_measure = 1e20;
int best_low_split_point = 0;
int best_high_split_point = 0;

double total_cost = get_cost();
double ideal_cost_per_chunk = total_cost / 3;

direction longest_axis = NO_DIRECTION;
int num_in_longest_axis = 0;
LOOP_OVER_DIRECTIONS(dim, d) {
if (num_direction(d) > num_in_longest_axis) {
longest_axis = d;
num_in_longest_axis = num_direction(d);
}
}

LOOP_OVER_DIRECTIONS(dim, d) {
double best_low_split_measure = 1e20;
for (int split_point = 1; split_point < num_direction(d); ++split_point) {
grid_volume v_low = *this;
v_low.set_num_direction(d, split_point);
double low_cost = v_low.get_cost();
double split_measure = fabs(low_cost - ideal_cost_per_chunk);

if (split_measure < best_low_split_measure) {
best_low_split_measure = split_measure;
best_low_split_point = split_point;
}
}

grid_volume low_gv = split_at_fraction(false, best_low_split_point, d, num_direction(d));
grid_volume high_two_gvs = split_at_fraction(true, best_low_split_point, d, num_direction(d));

double best_high_split_measure = 1e20;
for (int split_point = 1; split_point < high_two_gvs.num_direction(d); ++split_point) {
grid_volume v_low = high_two_gvs;
v_low.set_num_direction(d, split_point);
double low_cost = v_low.get_cost();
double split_measure = fabs(low_cost - ideal_cost_per_chunk);

if (split_measure < best_high_split_measure) {
best_high_split_measure = split_measure;
best_high_split_point = split_point;
}
}
grid_volume mid_gv = high_two_gvs.split_at_fraction(false, best_high_split_point, d,
high_two_gvs.num_direction(d));
grid_volume high_gv = high_two_gvs.split_at_fraction(true, best_high_split_point, d,
high_two_gvs.num_direction(d));

double low_cost = low_gv.get_cost();
double mid_cost = mid_gv.get_cost();
double high_cost = high_gv.get_cost();

double overall_split_measure = max(max(low_cost, mid_cost), high_cost);
bool within_thirty_percent = (overall_split_measure > best_overall_split_measure * 0.7 &&
overall_split_measure < best_overall_split_measure * 1.3);
bool at_least_thirty_percent_better = overall_split_measure < best_overall_split_measure * 0.7;

if ((within_thirty_percent && d == longest_axis) || at_least_thirty_percent_better) {
best_overall_split_measure = overall_split_measure;
best_low = low_gv;
best_mid = mid_gv;
best_high = high_gv;
}
}
result.push_back(best_low);
result.push_back(best_mid);
result.push_back(best_high);
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);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Collaborator

@stevengj stevengj Aug 1, 2019 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

At first glance, I think we should change two places:

  1. cost_diff should return right_cost - left_cost * (desired_chunks-1);

  2. find_best_split should set split_measure to max(left_cost * (desired_chunks-1), right_cost)

Copy link
Collaborator

@stevengj stevengj Aug 1, 2019 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.


grid_volume low_gv = split_at_fraction(false, best_low_split_point, best_low_split_direction,
num_direction(best_low_split_direction));
grid_volume high_two_gvs = split_at_fraction(true, best_low_split_point, best_low_split_direction,
num_direction(best_low_split_direction));

int best_high_split_point;
direction best_high_split_direction;
double right_effort_fraction;
high_two_gvs.find_best_split(2, best_high_split_point, best_high_split_direction, right_effort_fraction);

grid_volume mid_gv = high_two_gvs.split_at_fraction(false, best_high_split_point, best_high_split_direction,
high_two_gvs.num_direction(best_high_split_direction));
grid_volume high_gv = high_two_gvs.split_at_fraction(true, best_high_split_point, best_high_split_direction,
high_two_gvs.num_direction(best_high_split_direction));

result.push_back(low_gv);
result.push_back(mid_gv);
result.push_back(high_gv);
}

std::vector<grid_volume> grid_volume::split_into_n(int n) const {
Expand Down