buggy update

Author: GiovanniCanali

pina/domain/base_domain.py

@@ -1,5 +1,6 @@
 """Module for the Base Domain class."""
 
+from copy import deepcopy
 from .domain_interface import DomainInterface
 from ..utils import check_consistency, check_positive_integer
 
@@ -89,10 +90,12 @@ def update(self, domain):
         Each new label introduces a new dimension. Only domains of the same type
         can be used for update.
 
-        :param DomainInterface domain: The domain whose labels are to be merged
+        :param BaseDomain domain: The domain whose labels are to be merged
             into the current one.
         :raises TypeError: If the provided domain is not of the same type as
             the current one.
+        :return: A new domain instance with the merged labels.
+        :rtype: BaseDomain
         """
         # Raise an error if the domain types do not match
         if not isinstance(domain, type(self)):
@@ -102,8 +105,11 @@ def update(self, domain):
             )
 
         # Update fixed and ranged variables
-        self._fixed.update(domain._fixed)
-        self._range.update(domain._range)
+        updated = deepcopy(self)
+        updated._fixed.update(domain._fixed)
+        updated._range.update(domain._range)
+
+        return updated
 
     def _validate_sampling(self, n, mode, variables):
         """
@@ -164,7 +170,7 @@ def sample_modes(self, values):
         Setter for the ``sample_modes`` property.
 
         :param values: The sampling modes to be set.
-        :type values: str | list[str]
+        :type values: str | list[str] | tuple[str]
         :raises ValueError: Invalid sampling mode.
         """
         # Ensure values is a list

pina/domain/base_operation.py

@@ -1,5 +1,6 @@
 """Module for the Base Operation class."""
 
+from copy import deepcopy
 from .operation_interface import OperationInterface
 from .base_domain import BaseDomain
 from ..utils import check_consistency, check_positive_integer
@@ -31,26 +32,7 @@ def __init__(self, geometries):
         :raises NotImplementedError: If the dimensions of the geometries are not
             consistent.
         """
-        # Check geometries are list or tuple
-        if not isinstance(geometries, (list, tuple)):
-            raise TypeError(
-                "geometries must be either a list or a tuple of BaseDomain."
-            )
-
-        # Check consistency
-        check_consistency(geometries, (BaseDomain, BaseOperation))
-
-        # Check geometries
-        for geometry in geometries:
-            if geometry.variables != geometries[0].variables:
-                raise NotImplementedError(
-                    f"The {self.__class__.__name__} of geometries living in "
-                    "different ambient spaces is not well-defined. "
-                    "All geometries must share the same dimensions and labels."
-                )
-
-        # Initialization
-        self._geometries = geometries
+        self.geometries = geometries
 
     def _validate_sampling(self, n, mode, variables):
         """
@@ -95,18 +77,40 @@ def _validate_sampling(self, n, mode, variables):
 
         return sorted(variables)
 
-    def update(self, _):
+    def update(self, domain):
         """
         Update the domain resulting from the operation.
 
-        :raises NotImplementedError: The :meth:`update` method is not
-            implemented for operation domains. Please update the individual
-            domains instead.
-        """
-        raise NotImplementedError(
-            "The update method is not implemented for operation domains. "
-            "Please update the individual domains instead."
-        )
+        :param DomainInterface domain: The domain whose labels are to be merged
+            into the current one.
+        :raises NotImplementedError: If the geometries involved in the operation
+            are of different types.
+        :raises TypeError: If the passed domain is not of the same type of all
+            the geometries involved in the operation.
+        :return: A new domain instance with the merged labels.
+        :rtype: BaseOperation
+        """
+        # Check all geometries are of the same type
+        domain_type = type(self.geometries[0])
+        if not all(isinstance(g, domain_type) for g in self.geometries):
+            raise NotImplementedError(
+                f"The {self.__class__.__name__} of geometries of different"
+                " types does not support the update operation. All geometries"
+                " must be of the same type."
+            )
+
+        # Check domain type consistency
+        if not isinstance(domain, domain_type):
+            raise TypeError(
+                f"Cannot update the {self.__class__.__name__} of domains of"
+                f" type {domain_type} with domain of type {type(domain)}."
+            )
+
+        # Update each geometry
+        updated = deepcopy(self)
+        updated.geometries = [geom.update(domain) for geom in self.geometries]
+
+        return updated
 
     def partial(self):
         """
@@ -167,3 +171,36 @@ def geometries(self):
         :rtype: list[BaseDomain]
         """
         return self._geometries
+
+    @geometries.setter
+    def geometries(self, values):
+        """
+        Setter for the ``geometries`` property.
+
+        :param values: The geometries to be set.
+        :type values: list[BaseDomain] | tuple[BaseDomain]
+        :raises TypeError: If values is neither a list nor a tuple.
+        :raises ValueError: If values elements are not instances of
+            :class:`~pina.domain.base_domain.BaseDomain`.
+        :raises NotImplementedError: If the dimensions of the geometries are not
+            consistent.
+        """
+        # Check geometries are list or tuple
+        if not isinstance(values, (list, tuple)):
+            raise TypeError(
+                "geometries must be either a list or a tuple of BaseDomain."
+            )
+
+        # Check consistency
+        check_consistency(values, (BaseDomain, BaseOperation))
+
+        # Check geometries
+        for v in values:
+            if v.variables != values[0].variables:
+                raise NotImplementedError(
+                    f"The {self.__class__.__name__} of geometries living in "
+                    "different ambient spaces is not well-defined. "
+                    "All geometries must share the same dimensions and labels."
+                )
+
+        self._geometries = values

pina/domain/domain_interface.py

@@ -29,6 +29,8 @@ def update(self, domain):
 
         :param BaseDomain domain: The domain whose labels are to be merged into
             the current one.
+        :return: A new domain instance with the merged labels.
+        :rtype: DomainInterface
         """
 
     @abstractmethod
@@ -69,7 +71,7 @@ def sample_modes(self, values):
         Setter for the :attr:`sample_modes` property.
 
         :param values: Sampling modes to be set.
-        :type values: str | list[str]
+        :type values: str | list[str] | tuple[str]
         """
 
     @property

pina/domain/ellipsoid_domain.py

@@ -128,9 +128,13 @@ def update(self, domain):
             into the current one.
         :raises TypeError: If the provided domain is not of an instance of
             :class:`EllipsoidDomain`.
+        :return: A new domain instance with the merged labels.
+        :rtype: EllipsoidDomain
         """
-        super().update(domain)
-        self._compute_center_axes()
+        updated = super().update(domain)
+        updated._compute_center_axes()
+
+        return updated
 
     def sample(self, n, mode="random", variables="all"):
         """

pina/domain/operation_interface.py

@@ -18,3 +18,13 @@ def geometries(self):
         :return: The list of domains on which to perform the set operation.
         :rtype: list[BaseDomain]
         """
+
+    @geometries.setter
+    @abstractmethod
+    def geometries(self, values):
+        """
+        Setter for the ``geometries`` property.
+
+        :param values: The geometries to be set.
+        :type values: list[BaseDomain] | tuple[BaseDomain]
+        """

pina/domain/simplex_domain.py

@@ -113,6 +113,8 @@ def update(self, domain):
         :param SimplexDomain domain: The domain whose vertices are to be set
             into the current one.
         :raises TypeError: If the domain is not a :class:`SimplexDomain` object.
+        :return: A new domain instance with the merged labels.
+        :rtype: SimplexDomain
         """
         # Raise an error if the domain types do not match
         if not isinstance(domain, type(self)):
@@ -122,7 +124,10 @@ def update(self, domain):
             )
 
         # Replace geometry
-        self._vert_matrix = domain._vert_matrix
+        updated = deepcopy(self)
+        updated._vert_matrix = domain._vert_matrix
+
+        return updated
 
     def sample(self, n, mode="random", variables="all"):
         """

pina/problem/abstract_problem.py

@@ -4,7 +4,7 @@
 import warnings
 from copy import deepcopy
 from ..utils import check_consistency
-from ..domain import DomainInterface, CartesianDomain
+from ..domain import DomainInterface, CartesianDomain, OperationInterface
 from ..condition.domain_equation_condition import DomainEquationCondition
 from ..label_tensor import LabelTensor
 from ..utils import merge_tensors, custom_warning_format
@@ -288,7 +288,7 @@ def _apply_custom_discretization(self, sample_rules, domains):
                 "the input variables."
             )
         for domain in domains:
-            if not isinstance(self.domains[domain], CartesianDomain):
+            if not isinstance(self.domains[domain], (CartesianDomain, OperationInterface)):
                 raise RuntimeError(
                     "Custom discretisation can be applied only on Cartesian "
                     "domains"

pina/problem/zoo/advection.py

@@ -41,8 +41,8 @@ class AdvectionProblem(SpatialProblem, TimeDependentProblem):
     temporal_domain = CartesianDomain({"t": [0, 1]})
 
     domains = {
-        "D": CartesianDomain({"x": [0, 2 * torch.pi], "t": [0, 1]}),
-        "t0": CartesianDomain({"x": [0, 2 * torch.pi], "t": 0.0}),
+        "D": spatial_domain.update(temporal_domain),
+        "t0": spatial_domain.update(CartesianDomain({"t": 0})),
     }
 
     conditions = {

pina/problem/zoo/allen_cahn.py

@@ -45,8 +45,8 @@ class AllenCahnProblem(TimeDependentProblem, SpatialProblem):
     temporal_domain = CartesianDomain({"t": [0, 1]})
 
     domains = {
-        "D": CartesianDomain({"x": [-1, 1], "t": [0, 1]}),
-        "t0": CartesianDomain({"x": [-1, 1], "t": 0.0}),
+        "D": spatial_domain.update(temporal_domain),
+        "t0": spatial_domain.update(CartesianDomain({"t": 0})),
     }
 
     conditions = {

pina/problem/zoo/diffusion_reaction.py

@@ -47,15 +47,13 @@ class DiffusionReactionProblem(TimeDependentProblem, SpatialProblem):
     temporal_domain = CartesianDomain({"t": [0, 1]})
 
     domains = {
-        "D": CartesianDomain({"x": [-torch.pi, torch.pi], "t": [0, 1]}),
-        "g1": CartesianDomain({"x": -torch.pi, "t": [0, 1]}),
-        "g2": CartesianDomain({"x": torch.pi, "t": [0, 1]}),
-        "t0": CartesianDomain({"x": [-torch.pi, torch.pi], "t": 0.0}),
+        "D": spatial_domain.update(temporal_domain),
+        "boundary": spatial_domain.partial().update(temporal_domain),
+        "t0": spatial_domain.update(CartesianDomain({"t": 0})),
     }
 
     conditions = {
-        "g1": Condition(domain="g1", equation=FixedValue(0.0)),
-        "g2": Condition(domain="g2", equation=FixedValue(0.0)),
+        "boundary": Condition(domain="boundary", equation=FixedValue(0.0)),
         "t0": Condition(domain="t0", equation=Equation(initial_condition)),
     }