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
Enable Utilities::MPI::NoncontiguousPartitioner to handle padding #9639
Conversation
return it == indices_has.end() ? 0 : (*it + 1); | ||
[&indices_has_clean]() { | ||
const auto it = | ||
max_element(indices_has_clean.begin(), indices_has_clean.end()); |
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.
You'll want to use the following -- the current form may not work with some compilers.
max_element(indices_has_clean.begin(), indices_has_clean.end()); | |
std::max_element(indices_has_clean.begin(), indices_has_clean.end()); |
const auto it = | ||
max_element(indices_want.begin(), indices_want.end()); | ||
return it == indices_want.end() ? 0 : (*it + 1); | ||
max_element(indices_want_clean.begin(), indices_want_clean.end()); |
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.
same here
b30442e
to
804fb73
Compare
I have integrate @bangerth's suggestions, made a bug fix, and added a test. I think this PR is ready to be reviewed. |
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.
Looks good except for an unnecessary lambda (pre-existing, but it got worse with longer variable names).
const types::global_dof_index n_dofs = Utilities::MPI::max( | ||
std::max( | ||
[&indices_has]() { | ||
const auto it = max_element(indices_has.begin(), indices_has.end()); | ||
return it == indices_has.end() ? 0 : (*it + 1); | ||
[&indices_has_clean]() { | ||
const auto it = std::max_element(indices_has_clean.begin(), | ||
indices_has_clean.end()); | ||
return it == indices_has_clean.end() ? 0 : (*it + 1); | ||
}(), | ||
[&indices_want]() { | ||
const auto it = | ||
max_element(indices_want.begin(), indices_want.end()); | ||
return it == indices_want.end() ? 0 : (*it + 1); | ||
[&indices_want_clean]() { | ||
const auto it = std::max_element(indices_want_clean.begin(), | ||
indices_want_clean.end()); | ||
return it == indices_want_clean.end() ? 0 : (*it + 1); |
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 would be easier to read if you avoided the lambdas inside std::max
in favor of plain code here with temporary variable like types::global_dof_index max_has = ...
and max_wants
.
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.
I have made the change.
/rebuild |
804fb73
to
fcc2518
Compare
const types::global_dof_index local_n_dofs_has = [&indices_has_clean]() { | ||
const auto it = | ||
std::max_element(indices_has_clean.begin(), indices_has_clean.end()); | ||
return it == indices_has_clean.end() ? 0 : (*it + 1); | ||
}(); |
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.
We're almost there. I was thinking about this code and similar for the next block below:
const types::global_dof_index local_n_dofs_has = [&indices_has_clean]() { | |
const auto it = | |
std::max_element(indices_has_clean.begin(), indices_has_clean.end()); | |
return it == indices_has_clean.end() ? 0 : (*it + 1); | |
}(); | |
const types::global_dof_index local_n_dofs_has = indices_has_clean.empty() ? | |
0 : *std::max_element(indices_has_clean.begin(), indices_has_clean.end()); |
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.
I have made the change.
fcc2518
to
336eab7
Compare
Excellent, thank you! |
This PR enables
Utilities::MPI::NoncontiguousPartitioner
to handle padding in the source and destination vectors. The user can indicate that the purpose of a vector entry is only for padding by insertingnumbers::invalid_dof_index
intostd::vector<types::global_dof_index> &indices_has
and/orstd::vector<types::global_dof_index> &indices_want
.I still need to write an appropriate test.
FYI @nfehn @kronbichler