Refactor cells #168
Refactor cells #168
Conversation
| class AbstractCell(UFLObject): | ||
| """A base class for all cells.""" | ||
| @abstractmethod | ||
| def topological_dimension(self) -> int: |
There was a problem hiding this comment.
Could we make things properties rather than methods where appropriate?
There was a problem hiding this comment.
I was planning to do that in a separate PR.
I was aiming for this PR to need no changes (or negligible changes) in FFCx, DOLFINx, TSFC, and Firedrake so we could get it merged more easily and I can start experimenting with whether its useful to implement cells in the Basix UFL wrapper.
| def __lt__(self, other: AbstractCell) -> bool: | ||
| """Define an arbitrarily chosen but fixed sort order for all cells.""" | ||
| if type(self) == type(other): | ||
| s = (self.geometric_dimension(), self.topological_dimension()) | ||
| o = (other.geometric_dimension(), other.topological_dimension()) | ||
| if s != o: | ||
| return s < o | ||
| return self._lt(other) | ||
| else: | ||
| if type(self).__name__ == type(other).__name__: | ||
| raise ValueError("Cannot order cell types with the same name") | ||
| return type(self).__name__ < type(other).__name__ |
There was a problem hiding this comment.
Could one define a total order over cells given information about the cell complex they represent? Or is that insufficient because you might have two different cell implementations that both represent a hexahedron (say).
There was a problem hiding this comment.
you might have two different cell implementations that both represent a hexahedron (say).
Exactly that
There was a problem hiding this comment.
Is it sufficient (for sorting) to only define __lt__ or does one need to provide __eq__ and then @functools.total_ordering ?
There was a problem hiding this comment.
Good question. __eq__ is defined as part of UFLObject (formerly @attach_operators_from_hash_data). Only this and __lt__ were previously defined so I guess it's enough for what UFL currently does, but adding @functools.total_ordering probably isn't a bad idea
| __all_classes__ = ["AbstractCell", "Cell", "TensorProductCell"] | ||
|
|
||
|
|
||
| # --- The most abstract cell class, base class for other cell types | ||
| class AbstractCell(UFLObject): |
There was a problem hiding this comment.
My usual preferred approach these days for abstract interfaces is typing.Protocol rather than ABC. That said, I think because you have implementation in this class and third-party libraries will inherit from it, Protocol is not necessarily better.
WDYT?
There was a problem hiding this comment.
In the element class, I think it has to be an ABC for now, as there's some implementation in the base class.
Long term, we should maybe aim for Protocols, but for now I think it's confusing to have a mixture of the two, so I'm leaning towards making them ABCs for now
|
@mscroggs is this almost ready? |
Yes, I think this one is ready |
The deprecation was introduced 4 months ago (#168). Since all the core objects of UFL is using it (Mesh and FunctionSpace) it means that pytest is throwing tons of deprecation warnings at import: ```python python3 -W error::DeprecationWarning -c "import ufl.Mesh" Traceback (most recent call last): File "<string>", line 1, in <module> File "/root/shared/ufl/__init__.py", line 265, in <module> from ufl.domain import as_domain, AbstractDomain, Mesh, MeshView, TensorProductMesh File "/root/shared/ufl/domain.py", line 190, in <module> class TensorProductMesh(AbstractDomain): File "/root/shared/ufl/core/ufl_type.py", line 56, in attach_operators_from_hash_data warnings.warn("attach_operators_from_hash_data deprecated, please use UFLObject instead.", DeprecationWarning) DeprecationWarning: attach_operators_from_hash_data deprecated, please use UFLObject instead. ```
) * Remove deprecated functionality (attach_operators_from_hash_data). The deprecation was introduced 4 months ago (#168). Since all the core objects of UFL is using it (Mesh and FunctionSpace) it means that pytest is throwing tons of deprecation warnings at import: ```python python3 -W error::DeprecationWarning -c "import ufl.Mesh" Traceback (most recent call last): File "<string>", line 1, in <module> File "/root/shared/ufl/__init__.py", line 265, in <module> from ufl.domain import as_domain, AbstractDomain, Mesh, MeshView, TensorProductMesh File "/root/shared/ufl/domain.py", line 190, in <module> class TensorProductMesh(AbstractDomain): File "/root/shared/ufl/core/ufl_type.py", line 56, in attach_operators_from_hash_data warnings.warn("attach_operators_from_hash_data deprecated, please use UFLObject instead.", DeprecationWarning) DeprecationWarning: attach_operators_from_hash_data deprecated, please use UFLObject instead. ``` * Flake8 * implement __str__ for function spaces --------- Co-authored-by: Matthew Scroggs <[email protected]>
Make cells into abstract base classes so cells can be implemented easily outside UFL is desired.
(They may wish do this if, for example, if someone wanted to define a hexagon cell or any other cell not implemented in UFL directly.)
This does not change the current user interface or remove any available functionality, aside from removing a few dicts that no-one should've been using anyway.
This is coupled with FEniCS/ffcx#571