QuadratureSpace and FinitelElementSpace discrepancy for GetVDim

[jandrej]

QuadratureSpace and FinitelElementSpace discrepancy for GetVDim

[pazner]

I don't think it makes sense for QuadratureSpace to have a vdim. It's common to need many different vdim quantities at the same quadrature points. For example, you might have matrices, vectors, and scalars all living at the same quadrature points — it's convenient for all of these to share the same QuadratureSpace. The main purpose for QuadratureSpace is to associate to every element in the mesh a quadrature rule. The vdim of the fields is orthogonal to this concept.

While some of the same reasoning also applied to FiniteElementSpace, that case is also a little bit different (it makes sense for the space to have vdim as part of the space so you can easily create operators with the correct dimensions, etc.).

[v-dobrev]

Both QuadratureFunction and GridFunction have access to the vdim -- isn't that sufficient? Can you elaborate how the current state "prevents a general interface"?

I intentionally did not include vdim in the QuadratureSpace -- that way QuadratureFunctions with different vdim can reuse the same QuadratureSpace. Changing this for FinitelElementSpace and GridFunction will break backward compatibility. Maybe something to consider for v5.0.

[v-dobrev]

One option that will be backward compatible is to introduce VectorQuadratureSpace -- it will contain a pointer to QuadratureSpace and variable vdim. Then we can add convenience constructors and other methods in QuadratureFunction that take VectorQuadratureSpace instead of QuadratureSpace and vdim. This way QuadratureSpace can still be reused for different vdim and we have one object that combines QuadratureSpace and vdim.

[jandrej]

One option that will be backward compatible is to introduce VectorQuadratureSpace -- it will contain a pointer to QuadratureSpace and variable vdim. Then we can add convenience constructors and other methods in QuadratureFunction that take VectorQuadratureSpace instead of QuadratureSpace and vdim. This way QuadratureSpace can still be reused for different vdim and we have one object that combines QuadratureSpace and vdim.

This would feel more confusing to me. We don't have a "VectorFunctionSpace" anywhere else.

Changing this for FinitelElementSpace and GridFunction will break backward compatibility. Maybe something to consider for v5.0.

I was thinking to change QuadratureSpace, not the other spaces.

Both QuadratureFunction and GridFunction have access to the vdim -- isn't that sufficient? Can you elaborate how the current state "prevents a general interface"?

When I create an interface based on spaces, e.g. using std::variant all other spaces can access the vdim.

Here's a specific example

int GetVDim(const FieldDescriptor &f)
{
   return std::visit([](auto && arg)
   {
      using T = std::decay_t<decltype(arg)>;
      if constexpr (std::is_same_v<T, const FiniteElementSpace *>)
      {
         return arg->GetVDim();
      }
      else if constexpr (std::is_same_v<T, const ParFiniteElementSpace *>)
      {
         return arg->GetVDim();
      }
      else if constexpr (std::is_same_v<T, const QuadratureSpace *>)
      {
         // return arg->GetVDim();
         return 1;
      }
      else
      {
         static_assert(always_false<T>, "can't use GetVDim on type");
      }
   }, f.data);
}

[v-dobrev]

Changing QuadratureSpace to have vdim is not going to be backward compatible.

VectorQuadratureSpace is one option for the name -- we can call it something else. If you just don't like having this additional class, then I don't know -- do you have any other ideas?

By the way, we already have multiple "QuadratureSpace" classes: one base class,QuadratureSpaceBase, and two derived classes: QuadratureSpace and FaceQuadratureSpace. We probably want a class combining QuadratureSpaceBase pointer/reference with vdim.

Another option will be if you can change your GetVDim() function to take {Grid,ParGrid,Quadrature}Function as input argument.

[jandrej]

Another option will be if you can change your GetVDim() function to take {Grid,ParGrid,Quadrature}Function as input argument.

That's what I tried to get rid of because it communicates wrong semantics in my interface. It could make the impression that data from these is being used whereas it only serves as a description for dimensions.

[v-dobrev]

If we were to re-design the FiniteElementSpace -- I'd probably split it into two classes similar to what I suggested above for QuadratureSpace and VectorQuadratureSpace. The new "FiniteElementSpace" will be just scalar and the "VectorFiniteElementSpace", in addition to "FiniteElementSpace" pointer/reference and vdim, will have the "ordering" parameter.

We can even generalize this to "TensorFiniteElementSpace" where there are multiple dimensions (not just one vdim) and more general layouts for the tensor dofs ordering.