Use std::variant instead of a hand-rolled version. #16355
Conversation
| std::optional<typename Triangulation<dim, spacedim>::cell_iterator> cell; | ||
| const DoFHandler<dim, spacedim> *dof_handler; | ||
| bool level_dof_access; | ||
| /** | ||
| * The cell in question, if one has been assigned to this object. The | ||
| * concrete data type can either be a Triangulation cell iterator, a | ||
| * DoFHandler cell iterator, or a DoFHandler level cell iterator. | ||
| */ | ||
| std::optional< | ||
| std::variant<typename Triangulation<dim, spacedim>::cell_iterator, | ||
| typename DoFHandler<dim, spacedim>::cell_iterator, | ||
| typename DoFHandler<dim, spacedim>::level_cell_iterator>> | ||
| cell; |
There was a problem hiding this comment.
This is the key part of the patch. We really do store a variant already, we just break the data stored into the base object plus potentially the members of a derived class. Using the std::variant is, in my view, a clear win.
| case 1: | ||
| return std::get<1>(cell.value())->get_dof_handler().n_dofs(); | ||
| case 2: | ||
| return std::get<2>(cell.value())->get_dof_handler().n_dofs(); |
There was a problem hiding this comment.
Unrelated to this patch, but shouldn't we return
| return std::get<2>(cell.value())->get_dof_handler().n_dofs(); | |
| return std::get<2>(cell.value())->get_dof_handler().n_dofs(std::get<2>(cell.value())->level()); |
i.e., the number of level dofs rather than the number of dofs globally?
There was a problem hiding this comment.
That's not what the code originally did, nor what is apparently necessary to pass the test suite. This function is used in contexts such as this:
template <int dim, int spacedim>
template <typename Number>
void
FEValuesBase<dim, spacedim>::get_function_values(
const ReadVector<Number> &fe_function,
std::vector<Number> &values) const
{
Assert(this->update_flags & update_values,
ExcAccessToUninitializedField("update_values"));
AssertDimension(fe->n_components(), 1);
Assert(present_cell.is_initialized(), ExcNotReinited());
AssertDimension(fe_function.size(), present_cell.n_dofs_for_dof_handler());
// get function values of dofs on this cell
Vector<Number> dof_values(dofs_per_cell);
present_cell.get_interpolated_dof_values(fe_function, dof_values);
We pass in a global vector and interpolate it onto the current cell (active or not).
There was a problem hiding this comment.
In other words, I believe that the change I made is correct.
There was a problem hiding this comment.
Thanks for checking, this is not what I expected to happen, but I do see it now.
|
In reference to #16346. |
Follow-up to #16350.