Repository: pybamm-team/PyBaMM

Issue:
Fixes for Geometry class
- [ ] update notebooks/examples to use Geometry class
- [ ] add `add_domain` function
- [ ] fill in Geometry documentation

Discussion:

Patch:
diff --git a/examples/notebooks/create-model.ipynb b/examples/notebooks/create-model.ipynb
index c5f587282..19b53871b 100644
--- a/examples/notebooks/create-model.ipynb
+++ b/examples/notebooks/create-model.ipynb
@@ -538,7 +538,8 @@
    "source": [
     "# define geometry\n",
     "x = pybamm.SpatialVariable(\"x\", domain=\"SEI layer\", coord_sys=\"cartesian\")\n",
-    "geometry = {\"SEI layer\": {\"primary\": {x: {\"min\": pybamm.Scalar(0), \"max\": pybamm.Scalar(1)}}}}"
+    "geometry = pybamm.Geometry()\n",
+    "geometry.add_domain(\"SEI layer\", {\"primary\": {x: {\"min\": pybamm.Scalar(0), \"max\": pybamm.Scalar(1)}}})"
    ]
   },
   {
@@ -580,7 +581,7 @@
     {
      "data": {
       "text/plain": [
-       "<pybamm.models.base_model.BaseModel at 0x7f662c5055f8>"
+       "<pybamm.models.base_model.BaseModel at 0x7f634b6c3400>"
       ]
      },
      "execution_count": 17,
@@ -636,13 +637,13 @@
   },
   {
    "cell_type": "code",
-   "execution_count": 20,
+   "execution_count": 22,
    "metadata": {},
    "outputs": [
     {
      "data": {
       "application/vnd.jupyter.widget-view+json": {
-       "model_id": "954e4883b8ae4be6979962b15f33efb3",
+       "model_id": "ff4cc73b0ec7462d9428558c54cb4f1b",
        "version_major": 2,
        "version_minor": 0
       },
@@ -686,13 +687,6 @@
    "source": [
     "The purpose of this notebook has been to go through the steps involved in getting a simple model working within PyBaMM. However, if you plan on reusing your model and want greater flexibility then we recommend that you create a new class for your model. We have set out instructions on how to do this in the \"Adding a Model\" tutorial in the documentation. "
    ]
-  },
-  {
-   "cell_type": "code",
-   "execution_count": null,
-   "metadata": {},
-   "outputs": [],
-   "source": []
   }
  ],
  "metadata": {
diff --git a/pybamm/geometry/geometry.py b/pybamm/geometry/geometry.py
index fadb82733..bb7389b8c 100644
--- a/pybamm/geometry/geometry.py
+++ b/pybamm/geometry/geometry.py
@@ -5,22 +5,60 @@ import pybamm
 
 
 class Geometry(dict):
-    """A geometry class to store the details features of the cell geometry.
-        Geometry extends the class dictionary and uses the key words:
-        "negative electrode", "positive electrode", etc to indicate the subdomain.
-        Within each subdomain, there are "primary" and "secondary" dimensions.
-        "primary" dimensions correspond to dimensiones on which spatial
-        operators will be applied (e.g. the gradient and divergence). In contrast,
-        spatial operators do not act along "secondary" dimensions. This allows for
-        multiple independent particles to be included into a model.
 
-     **Extends**: :class:`dict`
+    """
+    A geometry class to store the details features of the cell geometry.
+
+    Geometry extends the class dictionary and uses the key words: "negative electrode",
+    "positive electrode", etc to indicate the subdomain.  Within each subdomain, there
+    are "primary", "secondary" or "tabs" dimensions.  "primary" dimensions correspond to
+    dimensions on which spatial operators will be applied (e.g. the gradient and
+    divergence). In contrast, spatial operators do not act along "secondary" dimensions.
+    This allows for multiple independent particles to be included into a model.
+
+    The values assigned to each domain are dictionaries containing the spatial variables
+    in that domain, along with expression trees giving their min and maximum extents.
+    For example, the following dictionary structure would represent a Geometry with a
+    single domain "negative electrode", defined using the variable `x_n` which has a
+    range from 0 to the pre-defined parameter `l_n`.
+
+    .. code-block:: python
+
+       {
+           "negative electrode": {
+               "primary": {x_n: {"min": pybamm.Scalar(0), "max": l_n}}
+           }
+       }
+
+    A user can create a new Geometry by combining one or more of the pre-defined
+    geometries defined with the names given below.
+
+    - "1D macro": macroscopic 1D cell geometry (i.e. electrodes)
+    - "3D macro": macroscopic 3D cell geometry
+    - "1+1D macro": 1D macroscopic cell geometry with a 1D current collector
+    - "1+2D macro": 1D macroscopic cell geometry with a 2D current collector
+    - "1D micro": 1D microscopic cell geometry (i.e. particles)
+    - "1+1D micro": This is the geometry used in the standard DFN or P2D model
+    - "(1+0)+1D micro": 0D macroscopic cell geometry with 1D current collector,
+                        along with the microscopic 1D particle geometry.
+    - "(2+0)+1D micro": 0D macroscopic cell geometry with 1D current collector,
+                        along with the microscopic 1D particle geometry.
+    - "(1+1)+1D micro": 1D macroscopic cell geometry, with 1D current collector model,
+                        along with the microscopic 1D particle geometry.
+    - "(2+1)+1D micro": 1D macroscopic cell geometry, with 2D current collector model,
+                        along with the microscopic 1D particle geometry.
+    - "2D current collector": macroscopic 2D current collector geometry
+
+    **Extends**: :class:`dict`
+
+    Parameters
+    ----------
 
-     Parameters
-     ----------
+    geometries: one or more strings or Geometry objects. A string will be assumed to be
+                one of the predefined Geometries given above
 
-     custom_geometry : dict containing any extra user defined geometry
-     """
+    custom_geometry : dict containing any extra user defined geometry
+    """
 
     def __init__(self, *geometries, custom_geometry={}):
         for geometry in geometries:
@@ -49,9 +87,66 @@ class Geometry(dict):
             # avoid combining geometries that clash
             if any([k in self.keys() for k in geometry.keys()]):
                 raise ValueError("trying to overwrite existing geometry")
-            self.update(geometry)
+
+            for k, v in geometry.items():
+                self.add_domain(k, v)
+
         # Allow overwriting with a custom geometry
-        self.update(custom_geometry)
+        for k, v in custom_geometry.items():
+            self.add_domain(k, v)
+
+    def add_domain(self, name, geometry):
+        """
+        Add a new domain to the geometry
+
+        Parameters
+        ----------
+
+        name: string giving the name of the domain
+
+        geometry: dict of variables in the domain, along with the minimum and maximum
+                extents (e.g. {"primary": {x_n: {"min": pybamm.Scalar(0), "max": l_n}}}
+        """
+        if not isinstance(name, str):
+            raise ValueError("name must be a string")
+
+        for k, v in geometry.items():
+            if k not in ["primary", "secondary", "tabs"]:
+                raise ValueError(
+                    "keys of geometry must be either \"primary\", \"secondary\" or "
+                    "\"tabs\""
+                )
+            if k != "tabs":
+                for variable, rnge in v.items():
+                    if not isinstance(variable, pybamm.SpatialVariable):
+                        raise ValueError(
+                            "inner dict of geometry must have pybamm.SpatialVariable "
+                            "as keys"
+                        )
+                    if list(rnge.keys()) != ["position"]:
+                        if "min" not in rnge.keys():
+                            raise ValueError(
+                                "no minimum extents for variable {}".format(variable)
+                            )
+                        if "max" not in rnge.keys():
+                            raise ValueError(
+                                "no maximum extents for variable {}".format(variable)
+                            )
+            else:
+                for region, params in v.items():
+                    if region not in ["negative", "positive"]:
+                        raise ValueError(
+                            "tabs region must be \"negative\" or \"positive\""
+
+                        )
+                    for pname in params.keys():
+                        if pname not in ["y_centre", "z_centre", "width"]:
+                            raise ValueError(
+                                "tabs region params must be \"y_centre\", "
+                                "\"z_centre\" or \"width\""
+                            )
+
+        self.update({name: geometry})
 
 
 class Geometry1DMacro(Geometry):
diff --git a/pybamm/processed_variable.py b/pybamm/processed_variable.py
index c7dbefa43..c7842ea10 100644
--- a/pybamm/processed_variable.py
+++ b/pybamm/processed_variable.py
@@ -195,6 +195,9 @@ class ProcessedVariable(object):
         elif self.domain == ["current collector"]:
             self.spatial_var_name = "z"
             self.z_sol = space
+        else:
+            self.spatial_var_name = "x"
+            self.x_sol = space
 
         # set up interpolation
         # note that the order of 't' and 'space' is the reverse of what you'd expect


Test patch:
diff --git a/tests/unit/test_geometry/test_geometry.py b/tests/unit/test_geometry/test_geometry.py
index 3c6daa6bc..b4030bc0e 100644
--- a/tests/unit/test_geometry/test_geometry.py
+++ b/tests/unit/test_geometry/test_geometry.py
@@ -167,6 +167,62 @@ class TestGeometryxp1p1DMicro(unittest.TestCase):
 
 
 class TestGeometry(unittest.TestCase):
+    def test_add_domain(self):
+        L = pybamm.Parameter('L')
+        zero = pybamm.Scalar(0)
+        x = pybamm.SpatialVariable('x', domain=["negative electrode"])
+        geometry = pybamm.Geometry()
+
+        geometry.add_domain("name_of_domain",
+                            {"primary": {x: {"min": zero, "max": L}}}
+                            )
+
+        geometry.add_domain("name_of_domain",
+                            {"tabs": {"negative": {"z_centre": L}}}
+                            )
+
+        # name must be a string
+        with self.assertRaisesRegex(ValueError, "name must be a string"):
+            geometry.add_domain(123,
+                                {"primary": {x: {"min": zero, "max": L}}}
+                                )
+
+        # keys of geometry must be either \"primary\" or \"secondary\"
+        with self.assertRaisesRegex(ValueError, "primary.*secondary"):
+            geometry.add_domain("name_of_domain",
+                                {"primaryy": {x: {"min": zero, "max": L}}}
+                                )
+
+        # inner dict of geometry must have pybamm.SpatialVariable as keys
+        with self.assertRaisesRegex(ValueError, "pybamm\.SpatialVariable as keys"):
+            geometry.add_domain("name_of_domain",
+                                {"primary": {L: {"min": zero, "max": L}}}
+                                )
+
+        # no minimum extents for variable
+        with self.assertRaisesRegex(ValueError, "minimum"):
+            geometry.add_domain("name_of_domain",
+                                {"primary": {x: {"max": L}}}
+                                )
+
+        # no maximum extents for variable
+        with self.assertRaisesRegex(ValueError, "maximum"):
+            geometry.add_domain("name_of_domain",
+                                {"primary": {x: {"min": zero}}}
+                                )
+
+        # tabs region must be \"negative\" or \"positive\"
+        with self.assertRaisesRegex(ValueError, "negative.*positive"):
+            geometry.add_domain("name_of_domain",
+                                {"tabs": {"negativee": {"z_centre": L}}}
+                                )
+
+        # tabs region params must be \"y_centre\", "\"z_centre\" or \"width\"
+        with self.assertRaisesRegex(ValueError, "y_centre.*z_centre.*width"):
+            geometry.add_domain("name_of_domain",
+                                {"tabs": {"negative": {"z_centree": L}}}
+                                )
+
     def test_combine_geometries(self):
         geometry1Dmacro = pybamm.Geometry1DMacro()
         geometry1Dmicro = pybamm.Geometry1DMicro()
diff --git a/tests/unit/test_processed_variable.py b/tests/unit/test_processed_variable.py
index 7600aeb0e..d17fa7ac9 100644
--- a/tests/unit/test_processed_variable.py
+++ b/tests/unit/test_processed_variable.py
@@ -55,6 +55,33 @@ class TestProcessedVariable(unittest.TestCase):
             x_s_edge.entries[:, 0], processed_x_s_edge.entries[1:-1, 0]
         )
 
+    def test_processed_variable_2D_unknown_domain(self):
+        x = pybamm.SpatialVariable("x", domain="SEI layer", coord_sys="cartesian")
+        geometry = pybamm.Geometry()
+        geometry.add_domain("SEI layer",
+                            {"primary":
+                                {x: {"min": pybamm.Scalar(0), "max": pybamm.Scalar(1)}}}
+                            )
+
+        submesh_types = {"SEI layer": pybamm.Uniform1DSubMesh}
+        var_pts = {x: 100}
+        mesh = pybamm.Mesh(geometry, submesh_types, var_pts)
+
+        nt = 100
+
+        solution = pybamm.Solution(
+            np.linspace(0, 1, nt),
+            np.zeros((var_pts[x], nt)),
+            np.linspace(0, 1, 1),
+            np.zeros((var_pts[x])),
+            "test"
+        )
+
+        c = pybamm.StateVector(slice(0, var_pts[x]), domain=["SEI layer"])
+        pybamm.ProcessedVariable(
+            c, solution.t, solution.y, mesh
+        )
+
     def test_processed_variable_3D(self):
         var = pybamm.Variable("var", domain=["negative particle"])
         x = pybamm.SpatialVariable("x", domain=["negative electrode"])