Refactor HangingNodes::setup_constraints() #12977
Conversation
peterrum
force-pushed
the
setup_constraints_refactor
branch
from
628b1c7
to
67c33fc
Compare
|
/rebuild |
| std::vector<std::vector<bool>> | ||
| compute_component_mask(const dealii::hp::FECollection<dim> &fe) const; | ||
|
|
||
| template <typename CellIterator> | ||
| ConstraintKinds | ||
| compute_refinement_configuration(const CellIterator &cell) const; | ||
|
|
||
| template <typename CellIterator> | ||
| void | ||
| update_dof_indices( | ||
| const CellIterator & cell, | ||
| const std::shared_ptr<const Utilities::MPI::Partitioner> &partitioner, | ||
| const std::vector<unsigned int> & lexicographic_mapping, | ||
| const std::vector<std::vector<bool>> &component_mask, | ||
| const ConstraintKinds & refinement_configuration, | ||
| std::vector<types::global_dof_index> &dof_indices) const; |
There was a problem hiding this comment.
Could you please document these functions? It is an internal class, but the other functions are.
| @@ -345,24 +368,139 @@ namespace internal | |||
|
|
|||
|
|
|||
|
|
|||
| template <int dim> | |||
| inline std::vector<std::vector<bool>> | |||
| HangingNodes<dim>::compute_component_mask( | |||
There was a problem hiding this comment.
Can you give this function a more expressive name, e.g.
| HangingNodes<dim>::compute_component_mask( | |
| HangingNodes<dim>::compute_supported_components( |
| // ignore neighbors that are artificial or have the same level | ||
| if (neighbor->is_artificial() || neighbor->level() == cell->level()) | ||
| continue; |
There was a problem hiding this comment.
It is a bit confusing to have the check for children (neighbor more refined) in a different places than the other case with the same property. How about moving all three cases involving the neighbor here?
| .tensor_degree() + | ||
| 1; | ||
| const unsigned int dofs_per_face = | ||
| Utilities::fixed_power<dim - 1>(n_dofs_1d); |
There was a problem hiding this comment.
nit-picking, but can you use
| Utilities::fixed_power<dim - 1>(n_dofs_1d); | |
| Utilities::pow(n_dofs_1d, dim - 1); |
| if ([](const auto &outer) { | ||
| for (const auto &inner : outer) | ||
| for (const auto i : inner) | ||
| if (i) | ||
| return true; | ||
| return false; | ||
| }(component_masks) == false) | ||
| return false; |
There was a problem hiding this comment.
I think this is hard to follow, why not simply
| if ([](const auto &outer) { | |
| for (const auto &inner : outer) | |
| for (const auto i : inner) | |
| if (i) | |
| return true; | |
| return false; | |
| }(component_masks) == false) | |
| return false; | |
| return std::none_of(component_mask.begin(), component_mask.end(), | |
| [] (const auto &a) { return *std::max_element(a.begin(), a.end());}); |
Or write out the loops to find an entry with true (at the expense of an additional variable as we can't return from within the loop.
|
@kronbichler I have made the changes. I also have added a test that the algorithm works for (2D) mixed meshes (with a few additional changes). |
peterrum
force-pushed
the
setup_constraints_refactor
branch
from
92b42dc
to
6c01b49
Compare
|
|
||
| // ignore neighbors that are artificial or have the same level or | ||
| // have children | ||
| if (cell->neighbor(face_no)->has_children() || |
There was a problem hiding this comment.
| if (cell->neighbor(face_no)->has_children() || | |
| if (neighbor->has_children() || |
peterrum
force-pushed
the
setup_constraints_refactor
branch
from
6c01b49
to
14a1c5b
Compare
With this PR, the algorithm is split up three steps:
dealii/include/deal.II/matrix_free/hanging_nodes_internal.h
Lines 730 to 755 in 4d3c409
(In follow up PRs, I would like to call the new functions at separate places to minimize redundant work).
The determination of the refinement configuration now consists of basic bit shifts of the form:
dealii/include/deal.II/matrix_free/hanging_nodes_internal.h
Line 439 in 4d3c409
dealii/include/deal.II/matrix_free/hanging_nodes_internal.h
Line 466 in 4d3c409
depends on
#12979, #12980, #1298