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
MGTwoLevelTransfer (p): add fallback FE case #12924
base: master
Are you sure you want to change the base?
Conversation
468444d
to
a0e819a
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like a good extension to me, but there are some questions I have regarding vector-valued cases (did we support them before)? See below.
const auto dummy_quadrature = | ||
reference_cell.template get_gauss_type_quadrature<dim>(1); | ||
internal::MatrixFreeFunctions::ShapeInfo<Number> shape_info; | ||
shape_info.reinit(dummy_quadrature, fe_fine, 0); | ||
lexicographic_numbering = shape_info.lexicographic_numbering; | ||
AssertThrow(false, ExcNotImplemented()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there an assumption about a single component somewhere else or are you now restricting to a smaller class of finite elements in the hypercube case? The original code of ShapeInfo
allowed multiple components, but you ask for n_components()==1
now, which is more restrictive than fe.n_base_elements()==1
I found as assertions elsewhere.
std::vector<unsigned int> renumbering(fe->n_dofs_per_cell()); | ||
{ | ||
AssertIndexRange(fe->n_dofs_per_vertex(), 2); | ||
renumbering[0] = 0; | ||
for (unsigned int i = 0; i < fe->dofs_per_line; ++i) | ||
renumbering[i + fe->n_dofs_per_vertex()] = | ||
GeometryInfo<1>::vertices_per_cell * | ||
fe->n_dofs_per_vertex() + | ||
i; | ||
if (fe->n_dofs_per_vertex() > 0) | ||
renumbering[fe->n_dofs_per_cell() - | ||
fe->n_dofs_per_vertex()] = | ||
fe->n_dofs_per_vertex(); | ||
} |
There was a problem hiding this comment.
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, but isn't this the regular hierarchic-to-lexicographic renumbering you would get from ShapeInfo::lexicographic_numbering
or maybe the inverse? I know it is a bit of a hammer to call that function, but this is not performance-critical code and 1D anyway, and I find it more documenting, and is what we do below.
else | ||
{ | ||
const auto dummy_quadrature = | ||
reference_cell.template get_gauss_type_quadrature<dim>(1); | ||
|
||
internal::MatrixFreeFunctions::ShapeInfo<Number> shape_info; | ||
shape_info.reinit(dummy_quadrature, | ||
dof_handler_fine.get_fe( | ||
fe_index_pair.first.second), | ||
0); | ||
lexicographic_numbering_fine[fe_index_pair.second] = | ||
shape_info.lexicographic_numbering; | ||
|
||
shape_info.reinit(dummy_quadrature, | ||
dof_handler_coarse.get_fe( | ||
fe_index_pair.first.first), | ||
0); | ||
lexicographic_numbering_coarse[fe_index_pair.second] = | ||
shape_info.lexicographic_numbering; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't see how this else
clause differs from the other places, except that it uses a different dummy quadrature. But we never use anything from the quadrature (as it is a dummy), so I don't see why we can't use the one from the if
clause also for non-hypercubes. Again, not the change in question here, but maybe we can fix it. If not, I would prefer to only set the quadrature formula in an if
/else
setup.
a0e819a
to
468444d
Compare
@peterrum what should we do about this PR? I suggest we move the target milestone, it seems like too much work the release now. |
@kronbichler I have removed the label. |
Hi @peterrum, I have reviewed this PR, however, given my limited familiarity with MGTransfers, I'm uncertain about the next steps. Could you please provide some guidance on how to proceed and complete it? |
... to allow more type of FEs (that are currently not supported by
MatrixFree
, e.g.RT
orNedelec
).Most changed lines are related to changed indentation (due to extra if-statements). These are the extra operations:
dealii/include/deal.II/multigrid/mg_transfer_global_coarsening.templates.h
Lines 1846 to 1866 in 468444d
dealii/include/deal.II/multigrid/mg_transfer_global_coarsening.templates.h
Lines 2130 to 2170 in 468444d
depends on #12923