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

Cleanups in some hp::DoFHandler functions. #7233

Merged
merged 1 commit into from Sep 27, 2018
Merged

Conversation

bangerth
Copy link
Member

This is in preparation for a patch to resolve #7162 after
@marcfehling explained the bug to me. That patch is coming
next but I thought I'd get the clean-ups out of the way
already given that these cleanups significantly obscure
what the real patch is going to be.

In pursuance of #3511.

@bangerth
Copy link
Member Author

/run-tests

@bangerth
Copy link
Member Author

Would appreciate a quick turn-around on this one.

Copy link
Member

@drwells drwells left a comment

Choose a reason for hiding this comment

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

Looks reasonable. I only have minor, cosmetic comments, so we can skip them if you are short on time.

// allocate offsets for all faces, though only the active
// ones will have a non-invalid value later on
face_dof_offsets =
std::vector<unsigned int>(dof_handler.tria->n_raw_faces(),
(unsigned int)(-1));
static_cast<unsigned int>(-1));
Copy link
Member

Choose a reason for hiding this comment

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

Would it be better to use numbers::invalid_unsigned_int here?

// 1 times dofs_per_face dofs, and one stop index
n_face_slots +=
1 +
dof_handler.get_fe(cell->active_fe_index())
.template n_dofs_per_object<dim - 1>() +
1;

// otherwise we do indeed need two sets, i.e. two
// Otherwise we do indeed need two sets, i.e. two
// active_fe_indices, two sets of dofs, and one stop
// index:
else
Copy link
Member

Choose a reason for hiding this comment

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

Would you mind adding the same comments here?

Copy link
Member

Choose a reason for hiding this comment

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

(that is, lines 454-460)

// allocate offsets for all faces, though only the active
// ones will have a non-invalid value later on
face_dof_offsets =
std::vector<unsigned int>(dof_handler.tria->n_raw_faces(),
(unsigned int)(-1));
Copy link
Contributor

Choose a reason for hiding this comment

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

Should not we use numbers::invalid_unsigned_int?

@drwells
Copy link
Member

drwells commented Sep 26, 2018

Also, if you are feeling fancy, we could use

for (const auto &face : triangulation.active_face_iterators())

instead of using user flags.

@bangerth
Copy link
Member Author

bangerth commented Sep 26, 2018 via email

@drwells
Copy link
Member

drwells commented Sep 26, 2018

Aw rats, you're right.

@bangerth
Copy link
Member Author

I made the requested changes and also created a small array to hold the two active_fe_index values in the second block that I edited. (You will see in the follow-up why.)

This passes all .*hp.* tests on my system.

// Store the two indices we will have to deal with.
unsigned int active_fe_indices[2] = {
cell->active_fe_index(),
cell->neighbor(face)->active_fe_index()};
Copy link
Member

Choose a reason for hiding this comment

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

👍

@drwells
Copy link
Member

drwells commented Sep 26, 2018

/run-tests

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.

Bug in hp::DoFHandler::distribute_dofs()
3 participants