Repository: pybamm-team/PyBaMM

Issue:
electrode tab coordinate inputs
**Summary**
 I think some of the example parameters such as LG cell provided with the software some electrode tab location numbers are not correct. 

For instance, 

Electrode height [m] = 0.065
Negative tab centre y-coordinate [m] = 0.06 <= does this mean the tab is located in the electrode coating? ( I might be confused). 
Would you direct me to the page where I have more information or easy pic to understand the coordinate system? 

![Capture](https://user-images.githubusercontent.com/60714526/79605900-beb69600-80be-11ea-957c-bc5f381c21c5.PNG)


Also, for the jellyroll structure, do I count for the entire length of the jellyroll?

 

Discussion:

Patch:
diff --git a/pybamm/geometry/geometry.py b/pybamm/geometry/geometry.py
index 4d38bd3bb..5b680f4dc 100644
--- a/pybamm/geometry/geometry.py
+++ b/pybamm/geometry/geometry.py
@@ -61,6 +61,8 @@ class Geometry(dict):
     """
 
     def __init__(self, *geometries, custom_geometry={}):
+        self._parameters = None
+
         for geometry in geometries:
             if geometry == "1D macro":
                 geometry = Geometry1DMacro()
@@ -95,6 +97,28 @@ class Geometry(dict):
         for k, v in custom_geometry.items():
             self.add_domain(k, v)
 
+    @property
+    def parameters(self):
+        "Returns all the parameters in the geometry"
+        if self._parameters is None:
+            self._parameters = self._find_parameters()
+        return self._parameters
+
+    def _find_parameters(self):
+        "Find all the parameters in the model"
+        unpacker = pybamm.SymbolUnpacker((pybamm.Parameter, pybamm.InputParameter))
+
+        def NestedDictValues(d):
+            "Get all the values from a nested dict"
+            for v in d.values():
+                if isinstance(v, dict):
+                    yield from NestedDictValues(v)
+                else:
+                    yield v
+
+        all_parameters = unpacker.unpack_list_of_symbols(list(NestedDictValues(self)))
+        return list(all_parameters.values())
+
     def add_domain(self, name, geometry):
         """
         Add a new domain to the geometry
diff --git a/pybamm/input/parameters/lithium-ion/cells/LGM50_Chen2020/parameters.csv b/pybamm/input/parameters/lithium-ion/cells/LGM50_Chen2020/parameters.csv
index 384342900..229e25e91 100644
--- a/pybamm/input/parameters/lithium-ion/cells/LGM50_Chen2020/parameters.csv
+++ b/pybamm/input/parameters/lithium-ion/cells/LGM50_Chen2020/parameters.csv
@@ -9,12 +9,6 @@ Positive electrode thickness [m],75.6E-6,Chen 2020,
 Positive current collector thickness [m],16E-6,Chen 2020,
 Electrode height [m],6.5E-2,Chen 2020,Not needed for 1D
 Electrode width [m],1.58,Chen 2020,Not needed for 1D
-Negative tab width [m],0.04,default,Need to find actual value for LG M50 cell
-Negative tab centre y-coordinate [m],0.06,default,Need to find actual value for LG M50 cell
-Negative tab centre z-coordinate [m],0.137,default,Need to find actual value for LG M50 cell
-Positive tab width [m],0.04,default,Need to find actual value for LG M50 cell
-Positive tab centre y-coordinate [m],0.147,default,Need to find actual value for LG M50 cell
-Positive tab centre z-coordinate [m],0.137,default,Need to find actual value for LG M50 cell
 ,,,
 # Current collector properties ,,,
 Negative current collector conductivity [S.m-1],58411000,CRC Handbook,copper
diff --git a/pybamm/input/parameters/lithium-ion/cells/UMBL_Mohtat2020/parameters.csv b/pybamm/input/parameters/lithium-ion/cells/UMBL_Mohtat2020/parameters.csv
index 915ea128c..bed5eb677 100644
--- a/pybamm/input/parameters/lithium-ion/cells/UMBL_Mohtat2020/parameters.csv
+++ b/pybamm/input/parameters/lithium-ion/cells/UMBL_Mohtat2020/parameters.csv
@@ -9,12 +9,6 @@ Positive electrode thickness [m],67E-06,Peyman MPM,
 Positive current collector thickness [m],2.5E-05,Scott Moura FastDFN,no info from Peyman MPM
 Electrode height [m],1,KOKAM SLPB78205130H,Not needed for 1D
 Electrode width [m],0.2050,KOKAM SLPB78205130H,Not needed for 1D
-Negative tab width [m],0.04,,Need to find actual value for KOKAM cell
-Negative tab centre y-coordinate [m],0.06,,Need to find actual value for KOKAM cell
-Negative tab centre z-coordinate [m],0.137,,Need to find actual value for KOKAM cell
-Positive tab width [m],0.04,,Need to find actual value for KOKAM cell
-Positive tab centre y-coordinate [m],0.147,,Need to find actual value for KOKAM cell
-Positive tab centre z-coordinate [m],0.137,,Need to find actual value for KOKAM cell
 ,,,
 # Current collector properties ,,,
 Negative current collector conductivity [S.m-1],59600000,LIONSIMBA,carbon
diff --git a/pybamm/models/base_model.py b/pybamm/models/base_model.py
index 44f5cf29e..0776e40ba 100644
--- a/pybamm/models/base_model.py
+++ b/pybamm/models/base_model.py
@@ -115,6 +115,7 @@ class BaseModel(object):
         self._jacobian = None
         self._jacobian_algebraic = None
         self.external_variables = []
+        self._parameters = None
         self._input_parameters = None
 
         # Default behaviour is to use the jacobian and simplify
@@ -284,6 +285,25 @@ class BaseModel(object):
         "Set the timescale"
         self._timescale = value
 
+    @property
+    def parameters(self):
+        "Returns all the parameters in the model"
+        if self._parameters is None:
+            self._parameters = self._find_parameters()
+        return self._parameters
+
+    def _find_parameters(self):
+        "Find all the parameters in the model"
+        unpacker = pybamm.SymbolUnpacker((pybamm.Parameter, pybamm.InputParameter))
+        all_parameters = unpacker.unpack_list_of_symbols(
+            list(self.rhs.values())
+            + list(self.algebraic.values())
+            + list(self.initial_conditions.values())
+            + list(self.variables.values())
+            + [event.expression for event in self.events]
+        )
+        return list(all_parameters.values())
+
     @property
     def input_parameters(self):
         "Returns all the input parameters in the model"


Test patch:
diff --git a/tests/unit/test_geometry/test_geometry.py b/tests/unit/test_geometry/test_geometry.py
index 0cea2d1ff..eff357b31 100644
--- a/tests/unit/test_geometry/test_geometry.py
+++ b/tests/unit/test_geometry/test_geometry.py
@@ -326,6 +326,50 @@ class TestGeometry2DCurrentCollector(unittest.TestCase):
                 )
 
 
+class TestReadParameters(unittest.TestCase):
+    # This is the most complicated geometry and should test the parameters are
+    # all returned for the deepest dict
+    def test_read_parameters(self):
+        L_n = pybamm.geometric_parameters.L_n
+        L_s = pybamm.geometric_parameters.L_s
+        L_p = pybamm.geometric_parameters.L_p
+        L_y = pybamm.geometric_parameters.L_y
+        L_z = pybamm.geometric_parameters.L_z
+        tab_n_y = pybamm.geometric_parameters.Centre_y_tab_n
+        tab_n_z = pybamm.geometric_parameters.Centre_z_tab_n
+        L_tab_n = pybamm.geometric_parameters.L_tab_n
+        tab_p_y = pybamm.geometric_parameters.Centre_y_tab_p
+        tab_p_z = pybamm.geometric_parameters.Centre_z_tab_p
+        L_tab_p = pybamm.geometric_parameters.L_tab_p
+
+        geometry = pybamm.Geometry("2+1D macro", "(2+1)+1D micro")
+
+        self.assertEqual(
+            set([x.name for x in geometry.parameters]),
+            set(
+                [
+                    x.name
+                    for x in [
+                        L_n,
+                        L_s,
+                        L_p,
+                        L_y,
+                        L_z,
+                        tab_n_y,
+                        tab_n_z,
+                        L_tab_n,
+                        tab_p_y,
+                        tab_p_z,
+                        L_tab_p,
+                    ]
+                ]
+            ),
+        )
+        self.assertTrue(
+            all(isinstance(x, pybamm.Parameter) for x in geometry.parameters)
+        )
+
+
 if __name__ == "__main__":
     print("Add -v for more debug output")
     import sys
diff --git a/tests/unit/test_models/test_base_model.py b/tests/unit/test_models/test_base_model.py
index 6e1d642e9..ba9a28462 100644
--- a/tests/unit/test_models/test_base_model.py
+++ b/tests/unit/test_models/test_base_model.py
@@ -114,6 +114,30 @@ class TestBaseModel(unittest.TestCase):
         self.assertEqual(model[key], rhs[key])
         self.assertEqual(model[key], model.rhs[key])
 
+    def test_read_parameters(self):
+        # Read parameters from different parts of the model
+        model = pybamm.BaseModel()
+        a = pybamm.Parameter("a")
+        b = pybamm.Parameter("b")
+        c = pybamm.Parameter("c")
+        d = pybamm.Parameter("d")
+        e = pybamm.Parameter("e")
+        f = pybamm.Parameter("f")
+
+        u = pybamm.Variable("u")
+        v = pybamm.Variable("v")
+        model.rhs = {u: -u * a}
+        model.algebraic = {v: v - b}
+        model.initial_conditions = {u: c, v: d}
+        model.events = [pybamm.Event("u=e", u - e)]
+        model.variables = {"v+f": v + f}
+
+        self.assertEqual(
+            set([x.name for x in model.parameters]),
+            set([x.name for x in [a, b, c, d, e, f]]),
+        )
+        self.assertTrue(all(isinstance(x, pybamm.Parameter) for x in model.parameters))
+
     def test_read_input_parameters(self):
         # Read input parameters from different parts of the model
         model = pybamm.BaseModel()
@@ -137,7 +161,7 @@ class TestBaseModel(unittest.TestCase):
             set([x.name for x in [a, b, c, d, e, f]]),
         )
         self.assertTrue(
-            all(isinstance(x, pybamm.InputParameter) for x in model.input_parameters),
+            all(isinstance(x, pybamm.InputParameter) for x in model.input_parameters)
         )
 
     def test_update(self):
@@ -287,7 +311,7 @@ class TestBaseModel(unittest.TestCase):
         model.rhs = {c: d.diff(pybamm.t), d: -1}
         model.initial_conditions = {c: 1, d: 1}
         with self.assertRaisesRegex(
-            pybamm.ModelError, "time derivative of variable found",
+            pybamm.ModelError, "time derivative of variable found"
         ):
             model.check_well_posedness()
 
@@ -296,7 +320,7 @@ class TestBaseModel(unittest.TestCase):
         model.algebraic = {c: 2 * d - c, d: c * d.diff(pybamm.t) - d}
         model.initial_conditions = {c: 1, d: 1}
         with self.assertRaisesRegex(
-            pybamm.ModelError, "time derivative of variable found",
+            pybamm.ModelError, "time derivative of variable found"
         ):
             model.check_well_posedness()
 
@@ -305,7 +329,7 @@ class TestBaseModel(unittest.TestCase):
         model.rhs = {c: d.diff(pybamm.t), d: -1}
         model.initial_conditions = {c: 1, d: 1}
         with self.assertRaisesRegex(
-            pybamm.ModelError, "time derivative of variable found",
+            pybamm.ModelError, "time derivative of variable found"
         ):
             model.check_well_posedness()
 
@@ -316,7 +340,7 @@ class TestBaseModel(unittest.TestCase):
             c: 5 * pybamm.StateVectorDot(slice(0, 15)) - 1,
         }
         with self.assertRaisesRegex(
-            pybamm.ModelError, "time derivative of state vector found",
+            pybamm.ModelError, "time derivative of state vector found"
         ):
             model.check_well_posedness(post_discretisation=True)
 
@@ -325,7 +349,7 @@ class TestBaseModel(unittest.TestCase):
         model.rhs = {c: 5 * pybamm.StateVectorDot(slice(0, 15)) - 1}
         model.initial_conditions = {c: 1}
         with self.assertRaisesRegex(
-            pybamm.ModelError, "time derivative of state vector found",
+            pybamm.ModelError, "time derivative of state vector found"
         ):
             model.check_well_posedness(post_discretisation=True)