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

Added hp::Refinement::copy_future_fe_indices(). #14173

Closed
wants to merge 1 commit into from

Conversation

marcfehling
Copy link
Member

Functions that copies future FE indices from one DoFHandler to another. Useful when dealing with multiple DoFHandler objects for the same problem, for example, if you have one for Stokes and one for the temperature as in step-32.

include/deal.II/hp/refinement.h Outdated Show resolved Hide resolved
@marcfehling
Copy link
Member Author

Found a typo. Now fixed.

Copy link
Member

@peterrum peterrum left a comment

Choose a reason for hiding this comment

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

A few stylists suggestions. One question: don't we need something like this also for actve_fe_index? What do you the first time?

Comment on lines +1069 to +1085

Assert(cell_destination->is_locally_owned(), ExcInternalError());
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
Assert(cell_destination->is_locally_owned(), ExcInternalError());

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 assertion is also in set_future_fe_index, so we really do not need it.

ExcMessage(
"Both DoFHandler objects must work on the same Triangulation!"));

typename DoFHandler<dim, spacedim>::active_cell_iterator cell_destination;
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
typename DoFHandler<dim, spacedim>::active_cell_iterator cell_destination;

if (cell_source->is_locally_owned() &&
cell_source->future_fe_index_set())
{
cell_destination =
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
cell_destination =
auto cell_destination =

Comment on lines +1055 to +1073
Assert(dof_handler_source.get_triangulation().n_active_cells() ==
dof_handler_destination.get_triangulation().n_active_cells(),
ExcMessage(
"Both DoFHandler objects must work on the same Triangulation!"));
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
Assert(dof_handler_source.get_triangulation().n_active_cells() ==
dof_handler_destination.get_triangulation().n_active_cells(),
ExcMessage(
"Both DoFHandler objects must work on the same Triangulation!"));
Assert(&dof_handler_source.get_triangulation() ==
&dof_handler_destination.get_triangulation(),
ExcMessage(
"Both DoFHandler objects must work on the same Triangulation!"));

Copy link
Member Author

Choose a reason for hiding this comment

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

Ah yes, comparing addresses is smarter :)

Copy link
Member Author

@marcfehling marcfehling left a comment

Choose a reason for hiding this comment

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

One question: don't we need something like this also for actve_fe_index? What do you the first time?

The workflow is usually that you only set active FE indices once in the beginning and manually. So if you have multiple DoFHandlers, you simply set the same indices on both. An additional function wouldn't be too useful I think.

The idea behind this function is that after you set future FE indices on one DoFHandler with one of the hp::Refinement functions, you can easily transfer the result to other DoFHandlers. This only makes sense for future FE indices in my opinion.

As an alternative, we could write {get|set}_future_fe_indices functions similar to DoFHandler::{get|set}_active_fe_indices. Then the user has to juggle with a container with all future FE indices. This might be a more versatile approach, and would form a pendant to the active FE indices functions.

I would be happy to transform this PR accordingly if you agree!

Comment on lines +1055 to +1073
Assert(dof_handler_source.get_triangulation().n_active_cells() ==
dof_handler_destination.get_triangulation().n_active_cells(),
ExcMessage(
"Both DoFHandler objects must work on the same Triangulation!"));
Copy link
Member Author

Choose a reason for hiding this comment

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

Ah yes, comparing addresses is smarter :)

Comment on lines +1069 to +1085

Assert(cell_destination->is_locally_owned(), ExcInternalError());
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 assertion is also in set_future_fe_index, so we really do not need it.

@peterrum
Copy link
Member

As an alternative, we could write {get|set}_future_fe_indices functions similar to DoFHandler::{get|set}_active_fe_indices. Then the user has to juggle with a container with all future FE indices. This might be a more versatile approach, and would form a pendant to the active FE indices functions.

Since there is already get_active_fe_indices()/set_active_fe_indices(), why not use the same approach here? The code in user code would not really longer: dof_handler_2.set_future_fe_indices(dof_handler_1.get_future_fe_indices());.

Personally, I like this suggestion 👍

@marcfehling
Copy link
Member Author

Superseded by #14208.

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

3 participants