Deprecate WithGeometry.create factory function (#4957)

Author: connorjward

docs/source/mesh-coordinates.rst

@@ -118,7 +118,7 @@ functions over to the new mesh.  To move `f` over to ``mesh``, use:
 
 .. code-block:: python3
 
-   g = Function(functionspaceimpl.WithGeometry.create(f.function_space(), mesh),
+   g = Function(functionspaceimpl.WithGeometry(f.function_space(), mesh),
                 val=f.topological)
 
 This creates a :py:class:`~.Function` `g` which shares data with `f`,

firedrake/checkpointing.py

@@ -1246,7 +1246,7 @@ def load_mesh(self, name=DEFAULT_MESH_NAME, reorder=None, distribution_parameter
                 radial_coord_name = self.get_attr(path, PREFIX + "_radial_coordinates")
                 radial_coordinates = self._load_function_topology(tmesh, radial_coord_element, radial_coord_name)
                 tV_radial_coord = impl.FunctionSpace(tmesh, radial_coord_element)
-                V_radial_coord = impl.WithGeometry.create(tV_radial_coord, mesh)
+                V_radial_coord = impl.WithGeometry(tV_radial_coord, mesh)
                 radial_coord_function_name = self.get_attr(path, PREFIX + "_radial_coordinate_function")
                 mesh.radial_coordinates = Function(V_radial_coord, val=radial_coordinates, name=radial_coord_function_name)
             # The followings are conceptually redundant, but needed.
@@ -1416,7 +1416,7 @@ def _load_function_space(self, mesh, name):
             element = self._load_ufl_element(path, PREFIX + "_ufl_element")
             tV = self._load_function_space_topology(tmesh, element)
             # Construct function space
-            V = impl.WithGeometry.create(tV, mesh)
+            V = impl.WithGeometry(tV, mesh)
         else:
             raise RuntimeError(f"""
                 FunctionSpace ({name}) not found in either of the following path in {self.filename}:

firedrake/functionspace.py

@@ -313,7 +313,7 @@ def rec(eles):
 
     mixed_mesh_geometry = MeshSequenceGeometry(meshes)
     new = impl.MixedFunctionSpace(spaces, mixed_mesh_geometry.topology, name=name)
-    return cls.create(new, mixed_mesh_geometry)
+    return cls(new, mixed_mesh_geometry)
 
 
 @PETSc.Log.EventDecorator("CreateFunctionSpace")
@@ -331,7 +331,7 @@ def RestrictedFunctionSpace(function_space, boundary_set=[], name=None):
         An optional name for the function space.
 
     """
-    return impl.WithGeometry.create(impl.RestrictedFunctionSpace(function_space,
-                                                                 boundary_set=boundary_set,
-                                                                 name=name),
-                                    function_space.mesh())
+    return impl.WithGeometry(impl.RestrictedFunctionSpace(function_space,
+                                                          boundary_set=boundary_set,
+                                                          name=name),
+                             function_space.mesh())

firedrake/functionspaceimpl.py

@@ -4,9 +4,8 @@
 classes for attaching extra information to instances of these.
 """
 
+import warnings
 from collections import OrderedDict
-from dataclasses import dataclass
-from typing import Optional
 
 import numpy
 
@@ -79,42 +78,57 @@ def check_element(element, top=True):
         check_element(e, top=False)
 
 
-class WithGeometryBase(object):
+class WithGeometryBase:
     r"""Attach geometric information to a :class:`~.FunctionSpace`.
 
     Function spaces on meshes with different geometry but the same
     topology can share data, except for their UFL cell.  This class
     facilitates that.
 
-    Users should not instantiate a :class:`WithGeometryBase` object
-    explicitly except in a small number of cases.
-
-    When instantiating a :class:`WithGeometryBase`, users should call
-    :meth:`WithGeometryBase.create` rather than ``__init__``.
+    Parameters
+    ----------
+    function_space : FunctionSpace or MixedFunctionSpace
+        Topological function space to attach geometry to.
+    mesh : MeshGeometry
+        Mesh with geometric information to use.
+    parent : WithGeometry
+        Parent geometric function space if exists.
 
-    :arg mesh: The mesh with geometric information to use.
-    :arg element: The UFL element.
-    :arg component: The component of this space in a parent vector
-        element space, or ``None``.
-    :arg cargo: :class:`FunctionSpaceCargo` instance carrying
-        Firedrake-specific data that is not required for code
-        generation.
     """
-    def __init__(self, mesh, element, component=None, cargo=None):
+    def __init__(self, function_space, mesh, parent=None):
+        if isinstance(function_space, MixedFunctionSpace):
+            if not isinstance(mesh, MeshSequenceGeometry):
+                raise TypeError(f"Can only use MixedFunctionSpace with MeshSequenceGeometry: got {type(mesh)}")
+
+        function_space = function_space.topological
+        assert mesh.topology == function_space.mesh()
+        assert mesh.topology != mesh
+
+        if function_space.parent is not None:
+            if parent is None:
+                raise ValueError("Must pass parent if function_space.parent is not None")
+        else:
+            parent = None
+
+        element = function_space.ufl_element().reconstruct(cell=mesh.ufl_cell())
         if type(element) is finat.ufl.MixedElement:
             if not isinstance(mesh, MeshSequenceGeometry):
                 raise TypeError(f"Can only use MixedElement with MeshSequenceGeometry: got {type(mesh)}")
-        assert component is None or isinstance(component, int)
-        assert cargo is None or isinstance(cargo, FunctionSpaceCargo)
-        super().__init__(mesh, element, label=cargo.topological._label or "")
-        self.component = component
-        self.cargo = cargo
+        assert function_space.component is None or isinstance(function_space.component, int)
+
+        self.topological = function_space
+        self.parent = parent
+        self.component = function_space.component
         self.comm = mesh.comm
+        super().__init__(mesh, element, label=function_space._label or "")
 
     @classmethod
     def create(cls, function_space, mesh, parent=None):
         """Create a :class:`WithGeometry`.
 
+        This factory function is deprecated. Use the `WithGeometry` constructor
+        instead.
+
         Parameters
         ----------
         function_space : FunctionSpace or MixedFunctionSpace
@@ -125,53 +139,22 @@ def create(cls, function_space, mesh, parent=None):
             Parent geometric function space if exists.
 
         """
-        if isinstance(function_space, MixedFunctionSpace):
-            if not isinstance(mesh, MeshSequenceGeometry):
-                raise TypeError(f"Can only use MixedFunctionSpace with MeshSequenceGeometry: got {type(mesh)}")
-        function_space = function_space.topological
-        assert mesh.topology == function_space.mesh()
-        assert mesh.topology != mesh
-
-        element = function_space.ufl_element().reconstruct(cell=mesh.ufl_cell())
-
-        topological = function_space
-        component = function_space.component
-
-        if function_space.parent is not None:
-            if parent is None:
-                raise ValueError("Must pass parent if function_space.parent is not None")
-        else:
-            parent = None
-
-        cargo = FunctionSpaceCargo(topological, parent)
-        return cls(mesh, element, component=component, cargo=cargo)
+        warnings.warn(
+            "'WithGeometry.create' is deprecated, instantiate them directly instead",
+            FutureWarning,
+        )
+        return cls(function_space, mesh, parent=parent)
 
     def _ufl_signature_data_(self, *args, **kwargs):
         return (type(self), self.component,
                 super()._ufl_signature_data_(*args, **kwargs))
 
-    @property
-    def parent(self):
-        return self.cargo.parent
-
-    @parent.setter
-    def parent(self, val):
-        self.cargo.parent = val
-
-    @property
-    def topological(self):
-        return self.cargo.topological
-
-    @topological.setter
-    def topological(self, val):
-        self.cargo.topological = val
-
     @cached_property
     def subspaces(self):
         r"""Split into a tuple of constituent spaces."""
         if isinstance(self.topological, MixedFunctionSpace):
             return tuple(
-                type(self).create(subspace, mesh, parent=self)
+                type(self)(subspace, mesh, parent=self)
                 for mesh, subspace in zip(self.mesh(), self.topological.subspaces, strict=True)
             )
         else:
@@ -193,7 +176,7 @@ def ufl_cell(self):
 
     @cached_property
     def _components(self):
-        return tuple(type(self).create(self.topological.sub(i), self.mesh(), parent=self)
+        return tuple(type(self)(self.topological.sub(i), self.mesh(), parent=self)
                      for i in range(self.block_size))
 
     @PETSc.Log.EventDecorator()
@@ -367,7 +350,7 @@ def boundary_nodes(self, sub_domain):
         return self._shared_data.boundary_nodes(self, sub_domain)
 
     def collapse(self):
-        return type(self).create(self.topological.collapse(), self.mesh())
+        return type(self)(self.topological.collapse(), self.mesh())
 
     @classmethod
     def make_function_space(cls, mesh, element, name=None):
@@ -395,7 +378,7 @@ def make_function_space(cls, mesh, element, name=None):
         # Skip this if we are just building subspaces of an abstract MixedFunctionSpace
         if mesh is not topology:
             # Create a concrete WithGeometry or FiredrakeDualSpace on this mesh
-            new = cls.create(new, mesh)
+            new = cls(new, mesh)
         return new
 
     def broken_space(self):
@@ -500,31 +483,21 @@ def reconstruct(
         return V
 
 
-class WithGeometry(WithGeometryBase, ufl.FunctionSpace):
-
-    def __init__(self, mesh, element, component=None, cargo=None):
-        super(WithGeometry, self).__init__(mesh, element,
-                                           component=component,
-                                           cargo=cargo)
+class WithGeometry(WithGeometryBase, ufl.functionspace.FunctionSpace):
 
     def dual(self):
         parent = None if self.parent is None else self.parent.dual()
-        return FiredrakeDualSpace.create(self.topological, self.mesh(), parent=parent)
+        return FiredrakeDualSpace(self.topological, self.mesh(), parent=parent)
 
 
 class FiredrakeDualSpace(WithGeometryBase, ufl.functionspace.DualSpace):
 
-    def __init__(self, mesh, element, component=None, cargo=None):
-        super(FiredrakeDualSpace, self).__init__(mesh, element,
-                                                 component=component,
-                                                 cargo=cargo)
-
     def dual(self):
         parent = None if self.parent is None else self.parent.dual()
-        return WithGeometry.create(self.topological, self.mesh(), parent=parent)
+        return WithGeometry(self.topological, self.mesh(), parent=parent)
 
 
-class FunctionSpace(object):
+class FunctionSpace:
     r"""A representation of a function space.
 
     A :class:`FunctionSpace` associates degrees of freedom with
@@ -1004,6 +977,7 @@ def __init__(self, function_space, boundary_set=frozenset(), name=None):
                                                      function_space.ufl_element(),
                                                      label=self._label)
         self.function_space = function_space
+        self.topological = self
         self.name = name or function_space.name
 
     def set_shared_data(self):
@@ -1301,7 +1275,7 @@ def __new__(cls, mesh, element, name=None):
         topology = mesh.topology
         self = super(ProxyFunctionSpace, cls).__new__(cls)
         if mesh is not topology:
-            return WithGeometry.create(self, mesh)
+            return WithGeometry(self, mesh)
         else:
             return self
 
@@ -1356,7 +1330,7 @@ def __new__(cls, function_space, boundary_set=frozenset(), name=None):
         topology = function_space._mesh.topology
         self = super(ProxyRestrictedFunctionSpace, cls).__new__(cls)
         if function_space._mesh is not topology:
-            return WithGeometry.create(self, function_space._mesh)
+            return WithGeometry(self, function_space._mesh)
         else:
             return self
 
@@ -1496,16 +1470,3 @@ def top_nodes(self):
     def local_to_global_map(self, bcs, lgmap=None, mat_type=None):
         assert len(bcs) == 0
         return None
-
-
-@dataclass
-class FunctionSpaceCargo:
-    """Helper class carrying data for a :class:`WithGeometryBase`.
-
-    It is required because it permits Firedrake to have stripped forms
-    that still know Firedrake-specific information (e.g. that they are a
-    component of a parent function space).
-    """
-
-    topological: FunctionSpace
-    parent: Optional[WithGeometryBase]

firedrake/mesh.py

@@ -2386,7 +2386,7 @@ def __init__(self, coordinates):
 
         # Save the coordinates as a 'CoordinatelessFunction' and as a 'Function'
         self._coordinates = coordinates
-        V = functionspaceimpl.WithGeometry.create(coordinates.function_space(), self)
+        V = functionspaceimpl.WithGeometry(coordinates.function_space(), self)
         self._coordinates_function = function.Function(V, val=coordinates)
 
     def _ufl_signature_data_(self, *args, **kwargs):
@@ -3197,7 +3197,7 @@ def make_vom_from_vom_topology(topology, name, tolerance=0.5):
         reference_coordinates = function.CoordinatelessFunction(reference_coordinates_fs,
                                                                 val=reference_coordinates_data,
                                                                 name=_generate_default_mesh_reference_coordinates_name(name))
-        refCoordV = functionspaceimpl.WithGeometry.create(reference_coordinates_fs, vmesh)
+        refCoordV = functionspaceimpl.WithGeometry(reference_coordinates_fs, vmesh)
         vmesh.reference_coordinates = function.Function(refCoordV, val=reference_coordinates)
     else:
         # We can't do this in 0D so leave it undefined.