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.

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

GC: enable inplace operation for coarse side #13494

Merged
merged 1 commit into from Mar 9, 2022

Conversation

peterrum
Copy link
Member

@peterrum peterrum commented Mar 5, 2022

first step towards reusing vectors

Comment on lines -1407 to -1412
if (mg_level_coarse == numbers::invalid_unsigned_int)
DoFTools::extract_locally_relevant_dofs(dof_handler_coarse,
locally_relevant_dofs);
else
DoFTools::extract_locally_relevant_level_dofs(
dof_handler_coarse, mg_level_coarse, locally_relevant_dofs);
Copy link
Member Author

Choose a reason for hiding this comment

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

This is a bit too conservative. I have changed it to active DoFs plus constraining DoFs.

@kronbichler
Copy link
Member

/rebuild

Copy link
Member

@kronbichler kronbichler left a comment

Choose a reason for hiding this comment

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

Nice. I think we should create a benchmark similar to the step-37 benchmark that picks up the global coarsening path with some hanging nodes (or also without), as this is a critical feature and having it among the benchmarks will raise awareness.

static std::shared_ptr<const Utilities::MPI::Partitioner>
create_coarse_partitioner(
const DoFHandler<dim> & dof_handler_coarse,
const dealii::AffineConstraints<Number> &constraint_coarse,
Copy link
Member

Choose a reason for hiding this comment

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

Not related to this PR, but it seems that this file uses the term constraint_coarse, whereas most other places would spell this as constraints_coarse (with the plural form).

Copy link
Member Author

Choose a reason for hiding this comment

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

Done!

const auto constraints =
constraint_coarse.get_constraint_entries(i);

if (constraints)
Copy link
Member

Choose a reason for hiding this comment

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

Isn't this check doing the same as the one in line 991 above?

Copy link
Member Author

Choose a reason for hiding this comment

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

I always had the impression that you need to call is_constrained() before get_constraint_entries(). But after taking a detailed look at:

const auto * entries_ptr =
constraints.get_constraint_entries(current_dof);
// dof is constrained
if (entries_ptr != nullptr)

I came to the realization that it is indeed not need 👍


if (constraints)
for (const auto &p : *constraints)
locally_relevant_dofs_temp.emplace_back(p.first);
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't it be possible to only add those elements that are not yet in locally_active_dofs? In that way, we would later simply append the indices to that previous list, rather than building and sorting a big list again.

Copy link
Member Author

Choose a reason for hiding this comment

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

I have changed it!

Copy link
Member

@kronbichler kronbichler left a comment

Choose a reason for hiding this comment

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

Looks great.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants