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

Select correct code path for neighbor elements in FEEvaluation #14903

Merged
merged 1 commit into from Mar 20, 2023

Conversation

bergbauer
Copy link
Contributor

No description provided.

@@ -3370,9 +3370,11 @@ FEEvaluationBase<dim, n_components_, Number, is_face, VectorizedArrayType>::
*this->dof_info);
}

const bool is_neighbor_cells = !is_face && !this->is_interior_face();
Copy link
Member

Choose a reason for hiding this comment

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

I know this is pre-existing due to #14119, but I must say that the name is really confusing. How about using use_unsorted_cells? (I am not super happy to have an additional statement in the critical path also for the contiguous storage case. I think we need to work on reducing overhead in the near future.)

Copy link
Member

Choose a reason for hiding this comment

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

You could also add an additional code comment. I had to look up in many places why we combine !is_face with information on a face, is_interior_face().

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think we could avoid this by setting this->cell = numbers::invalid_unsigned_int in FEEvaluationShift

this->cell = cell_batch_index;
which should choose the same code path as
reinit(const std::array<unsigned int, VectorizedArrayType::size()> &cell_ids);

Would you prefer this solution?

Copy link
Member

Choose a reason for hiding this comment

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

Yes, I think that would be better, as the information does not make sense anyway.

@kronbichler kronbichler merged commit 722c55a into dealii:master Mar 20, 2023
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