[go: up one dir, main page]
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.

Sign up for GitHub

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

Jump to bottom

External ufl types. #127

Open
dr-robertk opened this issue Oct 16, 2022 · 3 comments
Open

External ufl types. #127

dr-robertk opened this issue Oct 16, 2022 · 3 comments

Comments

@dr-robertk
Copy link
Contributor
dr-robertk commented Oct 16, 2022 •
edited
Loading

When adding an UFL type using the @ufl_type decorator this seems to lead to errors in, for example, the Transformer class from ufl.algorithms. Example, somewhere in some external (to ufl) code:

import ufl
from ufl.core.ufl_type import ufl_type
@ufl_type(is_scalar=True)
class BoundaryId(ufl.geometry.GeometricFacetQuantity):
    """UFL boundary id: can be used in a conditional to fix the desired boundary id."""
    __slots__ = ()
    name = "facetid"

This leads to an index error in

ufl/ufl/algorithms/transformer.py

Line 89 in 4e3868d

h, visit_children_first = self._handlers[o._ufl_typecode_]
It can be fixed by adding the new class manually to all_ufl_classes:

from ufl.classes import all_ufl_classes
all_ufl_classes.add(BoundaryId)

However, this can cause problems when other Python modules using UFL are included before these two lines, because it can mess up the index order in all_ufl_classes. So the result is, that all other modules using UFL should be included after this statement.

So the question is: What is the correct way of adding external UFL types (besides using @ufl_type)? Or is this a bug and the modification of the UFL internal lists and sets should be done inside the @ufl_type functions?
@dham
Copy link
Collaborator
dham commented Oct 16, 2022

Hi Rob,

This is actually a disgusting design flaw in UFL. It has its own type system layered on top of the Python one. I think this was done because at the time Python didn't have singledispatch. The effect of this is that it's essentially impossible to add UFL types outside of ufl itself, for exactly the reason you have noticed.

I think the correct answer is actually to get rid of the extra type system completely. We have funding to essentially rethink UFL and plan to do this, but it's going to take some time.

@dr-robertk
Copy link
Contributor Author

Hi David,

thanks for the quick answer. I was just wondering if in this particular case a quick fix could be implemented that updates the internal lists. It seems like the class variables in Expr._ufl_all_classes_ are updated correctly.
So at the end of

ufl/ufl/core/ufl_type.py

Line 215 in 4e3868d

def update_global_expr_attributes(cls):
the other lists could be updated too.

@mscroggs mscroggs assigned mscroggs and unassigned mscroggs Sep 12, 2023
@mscroggs
Copy link
Member

This will be fixed once we've removed the internal UFL type system

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
newFL
Status: No status
Milestone
No milestone
Development

No branches or pull requests
3 participants
@dham @dr-robertk @mscroggs