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

Fix interpolation for Arguments of zany spaces #286

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

wence-
Copy link
Contributor

@wence- wence- commented Jan 24, 2023

Previously we only checked if any coefficients in the source expression needed a coordinate mapping, but we should also check if any arguments do.

Previously we only checked if any coefficients in the source
expression needed a coordinate mapping, but we should also check if
any arguments do.
When determinining if the kernel needs a coordinate mapping we need to
check if terminal elements need it (e.g. VectorElement(Argyris)).
Previously we only checked the top-level element type which was
incorrect for this case.
@needs_coordinate_mapping.register(finat.HDivElement)
@needs_coordinate_mapping.register(finat.HCurlElement)
def _needs_coordinate_mapping_finat_hdivcurl(element):
return needs_coordinate_mapping(element.wrappee)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I put this here because it meant I didn't need a PR in finat as well, but probably finat should offer a traverse function that yields all the elements in a structured element. Everything needs special-casing because elements don't have regular "children" slots.

e.g. ideally you would just write (if you don't care about ordering)

def traverse(element):
    yield element
    yield from map(traverse, element.children)

expr = ufl.TestFunction(Q)
to_element = create_element(V.ufl_element())
kernel = compile_expression_dual_evaluation(expr, to_element, V.ufl_element())
assert kernel.needs_external_coords
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not checking for correctness, just that a kernel actually comes out the other end.

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.

1 participant