-
Notifications
You must be signed in to change notification settings - Fork 89
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
Fix block weights for fixed vertex partitioning #150
Conversation
@@ -233,6 +234,12 @@ static inline void partition(Hypergraph& input_hypergraph, | |||
HypergraphPtr(hg_without_fixed_vertices.first.release(), | |||
delete_hypergraph); | |||
fixed_vertex_free_to_input = hg_without_fixed_vertices.second; | |||
|
|||
for ( PartitionID block = 0; block < original_context.partition.k; ++block ) { |
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.
@kittobi1992 I might be missing something, but shouldn't this be something like
if (original_context.use_individual_part_weights) {
for ( PartitionID block = 0; block < original_context.partition.k; ++block ) {
rb_context.partition.max_part_weights[block] -= input_hypergraph.fixedVertexPartWeight(block);
}
}
rb_context.setupPartWeights(input_hypergraph_without_fixed_vertices->totalWeight());
so that we use the "normal" block-weight setup code whenever we don't use individual part weights?
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.
No, the fixed vertex block weights can be different for each block, which leads to individual block weights for initial Partitioning. If we do not use individual block weights, the part weight setup function will assign the same max part weight for each block.
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.
Ah, I see. That makes sense. Thanks for your explanation.
for ( PartitionID block = 0; block < original_context.partition.k; ++block ) { | ||
rb_context.partition.max_part_weights[block] -= input_hypergraph.fixedVertexPartWeight(block); | ||
} | ||
rb_context.partition.use_individual_part_weights = true; |
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.
This should also not be needed since we copy-construct the rb_context
from the original_context
, right?
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.
Should be answered with my previous comment. Even if we do not use individual block weights, removing fixed vertices leads to individual block weights for initial Partitioning.
Codecov Report
@@ Coverage Diff @@
## master #150 +/- ##
=======================================
Coverage 84.41% 84.42%
=======================================
Files 204 204
Lines 17792 17797 +5
Branches 9805 9807 +2
=======================================
+ Hits 15020 15025 +5
Misses 2772 2772
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
This PR addresses #148.
Quoting @kittobi1992: