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

Add check_restrictions in compute_form_data #34

Closed
wants to merge 2 commits into from

Conversation

cdaversin
Copy link
Contributor

Update existing check_restrictions code and use it in compute_form_data to limit usage of restrictions to interior facet and custom integral types.

error("Form argument must be restricted in interior facet integrals.")
else:
if self.current_restriction is not None:
error("Restrictions are only allowed for interior facet and custom integral types.")
Copy link
Collaborator

Choose a reason for hiding this comment

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

This hand-coded recursion in find_restriction unfortunately isn't a scalable approach [it's quadratic in tree depth]. Unfortunately, to fix this is a little bit of work, the map_dag infrastructure provides facility for DAG-based visiting and memoization of post-order traversal (child before parent), but not the preorder (parent before child) traversal we need here. I think this is a wider problem.

I think you can phrase this visit as post-order traversal, but it's a little fiddly.

Unrelated to this patch, reading this code I wondered how it works at all, since in def restricted it calls self.visit but visit is not defined anywhere.

I think this was not completely ported from the old ReuseTransformer infrastructure.

It's not previously been used in the normal pipeline (we have some error checking via apply_default_restrictions that is only applied to interior_facet integrals), so this is why we haven't ever noticed.

@miklos1 miklos1 mentioned this pull request Aug 25, 2020
@mscroggs mscroggs closed this Sep 12, 2023
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.

None yet

3 participants