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

MatrixFree: Do not construct hanging nodes structure for non-adapted meshes #13872

Merged
merged 1 commit into from Jun 1, 2022

Conversation

kronbichler
Copy link
Member

This reduces the initialization cost of a data structure that is added with this release, so I would like to tag it with 9.4. Note that this code is well-tested in around a hundred of tests for matrix-free computations.

bool all_cells_on_same_level = true;
for (const auto &cell : triangulation.active_cell_iterators())
if (cell->level() == static_cast<int>(triangulation.n_levels()) - 1)
break;
Copy link
Member

Choose a reason for hiding this comment

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

I don't understand, do you take advantage of the order the cells are traversed here? Also can you wrap the contents of the for loop in { }?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, I do: I do not want to loop through all cells, since it is enough to complete the loop through all active cells on a coarser level and see if there is one non-artificial. We maybe have an iterator filter, but after looking for 30 seconds I decided I was quicker to implement it. Do you want me to not assume the order of active cells level by level? I could do so by traversing all cells and removing this if clause here, at a probably negligible impact on performance.

// In case all our cells are on the same level, there is nothing to
// do. We only need to check for active cells (non-artificial) that are
// on a level coarser than the finest one.
bool all_cells_on_same_level = true;
Copy link
Member

Choose a reason for hiding this comment

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

Why not use Triangulation::has_hanging_nodes()?

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 don't want to go through the communication involved there, and wanted it to be a local operation.

Copy link
Member

Choose a reason for hiding this comment

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

Wouldn't it be better to move the information to number_cache?

Copy link
Member Author

Choose a reason for hiding this comment

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

At some point yes, but for the current release it feels too unsafe to touch that part. But let me use a similar code as the has_hanging_nodes function, to reduce the variability we have in the code base.

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 now pushed the update.

@masterleinad masterleinad merged commit 842b53c into dealii:master Jun 1, 2022
@kronbichler kronbichler deleted the reduce_cost_hanging_nodes branch September 5, 2022 12:10
mkghadban pushed a commit to OpenFCST/dealii that referenced this pull request Sep 8, 2022
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

4 participants