ufl icon indicating copy to clipboard operation
ufl copied to clipboard

External ufl types.

Open dr-robertk opened this issue 3 years ago • 3 comments

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 https://github.com/FEniCS/ufl/blob/4e3868dee951504484b7f85e2c0e477ef7894c23/ufl/algorithms/transformer.py#L89 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?

dr-robertk avatar Oct 16 '22 10:10 dr-robertk

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.

dham avatar Oct 16 '22 18:10 dham

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 https://github.com/FEniCS/ufl/blob/4e3868dee951504484b7f85e2c0e477ef7894c23/ufl/core/ufl_type.py#L215 the other lists could be updated too.

dr-robertk avatar Oct 17 '22 09:10 dr-robertk

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

mscroggs avatar Sep 12 '23 08:09 mscroggs