Skip to content

Conversation

@pbrubeck
Copy link
Contributor

Description

This enables variant and quad_scheme to be passed on to finat direclty from the FunctionSpace constructor.

I don't think that tests for quad_scheme are necessary at this point, as we already have tests for variant.

@pbrubeck pbrubeck force-pushed the pbrubeck/element-kwargs branch from e464470 to 3786c62 Compare September 30, 2025 14:21
@pbrubeck
Copy link
Contributor Author

We got an unrelated failure in parallel (no variants involved), maybe that's random. I'll rerun CI

Copy link
Contributor

@connorjward connorjward left a comment

Choose a reason for hiding this comment

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

Now that we know that some in the team don't like **kwargs too much I think we should let them review this. Personally I think this is fine since it means we won't need to change a bunch of functions every time a new option is added to the finite element constructor.

It would also be good for these to have type hints as there currently appears to be no type information for all of these very public functions.

@pbrubeck pbrubeck requested a review from connorjward October 13, 2025 13:05
Copy link
Contributor

@connorjward connorjward left a comment

Choose a reason for hiding this comment

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

I know it's a pain but I think that these functions really need to have type hints. There is no way that a user is going to know what should be passed as quad_scheme.

@pbrubeck
Copy link
Contributor Author

pbrubeck commented Oct 16, 2025

I know it's a pain but I think that these functions really need to have type hints. There is no way that a user is going to know what should be passed as quad_scheme.

I think this is the right place to open a discussion with @rckirby, @dham, @leo-collins, and anyone else who cares about FunctionSpaces that just hold data at points, or changing the quadrature rules in the degrees of freedom.

For quad_scheme we want to be able to pass three types:

  1. A str, it could be either "default" (pretabulated simplex rules), "canonical" (collapsing a hypercube into a simplex) or "KMV" (mass lumping), or a comma-separated list that enables for composite (macro) schemes, eg. "KMV,alfeld".
  2. A finat QuadratureRule to construct a "Quadrature" or "Boundary Quadrature" elements meant for holding data at integration points. See Boundary Quadrature elements #3973.
  3. A finat PointSet, to construct "Quadarature" elements meant for holding data at interpolation points. See Quadrature elements constructed from non-Lagrange elements fiat#187 where I am passing quad_scheme=V.finat_element, just because it is easier. If we want to attach any quadrature weights for this case, the safe bet is to set them with nan, since these points are not meant for integration.

Elements with degrees of freedom on facets of different dimension cannot really use a fixed QuadratureRule or a fixed PointSet to compute the dofs on entities of different dimensions, but they can extract the relevant information from a str to construct their own internal quadrature rules on the facets of the reference elements. See firedrakeproject/fiat#186.

Currently, we can pass quad_scheme via the finat.ufl.FiniteElement constructor, but not yet via FunctionSpace. This PR should add docstrings explaining what quad_scheme does.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants