Skip to content
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

Use subdegree instead of superdegree to check cell bendy-ness #295

Merged

Conversation

connorjward
Copy link
Contributor

Fixes firedrakeproject/firedrake#3612

To quote @dham:

The reason subdegree is right is that you care about whether the edges are straight, not whether there are any quadratic functions on the interior.

The reason subdegree is right is that you care about whether the
edges are straight, not whether there are any quadratic functions
on the interior.

See firedrakeproject/firedrake#3612.
@mscroggs
Copy link
Member

mscroggs commented Jul 2, 2024

I'm surprised this makes any difference, as for P and Q spaces, the embedded sub- and superdegrees are the same.

At least, they're the same for the elements coming from Basix. Maybe this isn't true for elements from FInAT?

@connorjward
Copy link
Contributor Author

I'm surprised this makes any difference, as for P and Q spaces, the embedded sub- and superdegrees are the same.

At least, they're the same for the elements coming from Basix. Maybe this isn't true for elements from FInAT?

They differ for tensor product elements. We encountered the issue on extruded meshes.

@wence-
Copy link
Collaborator

wence- commented Jul 2, 2024

At least, they're the same for the elements coming from Basix.

They shouldn't be for Q, no? e.g. Q1 on quadrilaterals has a basis $\text{span} \{1, x, y, x y \}$ which has an embedding superdegree of 2.

@mscroggs
Copy link
Member

mscroggs commented Jul 4, 2024

Embedded degrees are defined in terms of the span of Lagrange on the current cell:

ufl/ufl/finiteelement.py

Lines 70 to 83 in ea144b0

def embedded_superdegree(self) -> _typing.Union[int, None]:
"""Degree of the minimum degree Lagrange space that spans this element.
This returns the degree of the lowest degree Lagrange space such
that the polynomial space of the Lagrange space is a superspace
of this element's polynomial space. If this element contains
basis functions that are not in any Lagrange space, this
function should return None.
Note that on a simplex cells, the polynomial space of Lagrange
space is a complete polynomial space, but on other cells this is
not true. For example, on quadrilateral cells, the degree 1
Lagrange space includes the degree 2 polynomial xy.
"""

This is important for pyramids, where rational functions like $\frac{x}{1-z}$ are included in the polysets, so talking about standard polynomial degrees doesn't make sense.

@mscroggs
Copy link
Member

mscroggs commented Jul 4, 2024

Longer term, I think we should add information about the traces of elements (#298), as I think we should be asking here whether the traces on the edge are linear, which the embedded degrees don't really capture. But happy to merge this in the meantime, just updating TSFC branches to try to get those tests to pass...

@mscroggs mscroggs added this pull request to the merge queue Jul 4, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Jul 4, 2024
@mscroggs mscroggs enabled auto-merge July 4, 2024 08:07
@mscroggs mscroggs added this pull request to the merge queue Jul 4, 2024
Merged via the queue into FEniCS:main with commit e69de17 Jul 4, 2024
12 checks passed
@mscroggs mscroggs deleted the connorjward/fix-cell-diameter branch July 4, 2024 08:10
@wence-
Copy link
Collaborator

wence- commented Jul 4, 2024

Embedded degrees are defined in terms of the span of Lagrange on the current cell:

[...]

This is important for pyramids, where rational functions like x1−z are included in the polysets, so talking about standard polynomial degrees doesn't make sense.

I agree that if the basis is non-polynomial it doesn't make much sense.

However, my reading of the docstring you quoted still suggests that Q1 should be advertising an embedded_superdegree of 2. Since the complete polynomial space that spans the Q1 basis is of degree 2. Am I misunderstanding?

@mscroggs
Copy link
Member

mscroggs commented Jul 4, 2024

However, my reading of the docstring you quoted still suggests that Q1 should be advertising an embedded_superdegree of 2. Since the complete polynomial space that spans the Q1 basis is of degree 2. Am I misunderstanding?

My intended reading of "This returns the degree of the lowest degree Lagrange space such that the polynomial space of the Lagrange space is a superspace of this element's polynomial space" is "This returns the degree of the lowest degree Lagrange space on the cell on which this element is defined such that the polynomial space of the Lagrange space is a superspace of this element's polynomial space", ie for elements on quads, it returns the degree of the Lagrange/Q space in which it's embedded.

We were discussing this at PDESoft, and I was leaning towards your reading being better. But since thinking about pyramids I'm less sure, and remember why I made it this way.

@mscroggs
Copy link
Member

mscroggs commented Jul 4, 2024

Lagrange/Q space

Also this assumes that everyone overloads the name "Lagrange" in the same way I do which is in no way a safe assumption.

@wence-
Copy link
Collaborator

wence- commented Jul 4, 2024

Also this assumes that everyone overloads the name "Lagrange" in the same way I do which is in no way a safe assumption.

Yeah, I think this is probably one aspect of the confusion. I suspect it is better to talk in terms of spans of polynomial sets, rather than names for this.

But since thinking about pyramids I'm less sure, and remember why I made it this way.

AIUI (and I haven't really done anything with pyramids), if these have rational basis functions then there is no polynomial space that spans the basis. And so one should return None.

So I'm not quite sure how the "complete polynomial space" reading conflicts: there is no such space, so one returns None.

@mscroggs
Copy link
Member

mscroggs commented Jul 4, 2024

So I'm not quite sure how the "complete polynomial space" reading conflicts: there is no such space, so one returns None.

It can still be useful to know which "degree" the elements on pyramids are included in, for example for telling if the edges of the pyramid are straight.

Thought to return to later: If #298 is done, do we still need embedded degrees?

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.

BUG: CellDiameter on extruded meshes no longer works
3 participants