Repository: tinosulzer/PyBaMM

Issue:
ParameterValues class for a specific model
Building on issue #18 / PR #35 

For a particular, model, parameter values may be needed that are combinations of "raw" parameter values. For example, the total electrode length `L = Ln + Ls + Lp`, or any dimensionless parameter. 

It doesn't make sense to write these all in the csv file that gets imported, so there needs to be a class that calculates the "derived" parameters, maybe split by domain ("negative_electrode", "separator", "positive_electrode" or "whole_cell"). There may be overlap between the domains.
This class should also contain scales for non/re-dimensionalisation

For example,
```python3
class DiffusionParameterValues(BaseParameters):
    def __init__(self, base_parameters={}, optional_parameters={}):
       super().__init__(base_parameters, optional_parameters)
       
       self.scales = {"length": self.raw["Ln"] + self.raw["Ls"] + self.raw["Lp"],
                      "time": ...,
                      etc}
       self.whole_cell = {"L": self.raw["Ln"] + self.raw["Ls"] + self.raw["Lp"],
                          "Cd": self.scales["length"]**2/self.raw["D"]/self.scales["time"],
                          etc}
       self.negative_electrode = {"L": self.raw["Ln"], etc}
       self.separator = {"L": self.raw["Ls"], etc}
       self.positive_electrode = {"L": self.raw["Ln"], etc}
```
Maybe make `DiffusionParameterValues` emulate a dictionary, similarly to #19 , with keys being domain names.

Now if we want to include new parameters, say for a temperature model, we will need more scales and derived parameters, but these should be in a separate class as they might call upon `raw` values that would not necessarily be given to a `DiffusionParameterValues` class.

```python3
class TemperatureParameterValues(BaseParameters):
    def __init__(self, base_parameters={}, optional_parameters={}):
       super().__init__(base_parameters, optional_parameters)
       
       self.scales = {"Temp": self.raw["T_init"]}
       self.whole_cell = {"k": self.raw["D_temp"]/ bla bla}
       self.negative_electrode = {}
       self.separator = {}
       self.positive_electrode = {}
```
Then a `BatteryParameterValues` could multiply-inherit from `DiffusionParameterValues` and `TemperatureParameterValues`, combining the dictionary attributes from each of these (not sure how to do this)

@martinjrobins @Scottmar93 do you think this is a good approach? See [previous implementation](https://github.com/tinosulzer/PyBaMM/blob/structure/pybamm/parameters/parameter_values.py) (quite different) for an idea of which derived parameters need to be implemented.

Discussion:

Patch:
diff --git a/docs/source/parameters/index.rst b/docs/source/parameters/index.rst
index 3b04c1b6..925b3af5 100644
--- a/docs/source/parameters/index.rst
+++ b/docs/source/parameters/index.rst
@@ -3,4 +3,5 @@ Parameters
 
 .. toctree::
 
-  base_parameter_values
+  parameter_values
+  standard_parameters
diff --git a/docs/source/parameters/base_parameter_values.rst b/docs/source/parameters/parameter_values.rst
similarity index 57%
rename from docs/source/parameters/base_parameter_values.rst
rename to docs/source/parameters/parameter_values.rst
index 04ffb0f4..2ea2d675 100644
--- a/docs/source/parameters/base_parameter_values.rst
+++ b/docs/source/parameters/parameter_values.rst
@@ -1,5 +1,5 @@
 Base Parameter Values
 =====================
 
-.. autoclass:: pybamm.BaseParameterValues
+.. autoclass:: pybamm.ParameterValues
   :members:
diff --git a/docs/source/parameters/standard_parameters.rst b/docs/source/parameters/standard_parameters.rst
new file mode 100644
index 00000000..d5b076ef
--- /dev/null
+++ b/docs/source/parameters/standard_parameters.rst
@@ -0,0 +1,4 @@
+Standard Parameters
+===================
+
+.. automodule:: pybamm.parameters.standard_parameters
diff --git a/examples/notebooks/simple-diffusion.ipynb b/examples/notebooks/simple-diffusion.ipynb
index b7fcef93..53420fb9 100644
--- a/examples/notebooks/simple-diffusion.ipynb
+++ b/examples/notebooks/simple-diffusion.ipynb
@@ -67,7 +67,7 @@
    "cell_type": "markdown",
    "metadata": {},
    "source": [
-    "We then load parameters for the model from [pybamm.BaseParameterValues](); passing no arguments will use the default parameter values defined in [input/parameters/default.csv](https://github.com/tinosulzer/PyBaMM/blob/master/input/parameters/default.csv)"
+    "We then load parameters for the model from [pybamm.ParameterValues](); passing no arguments will use the default parameter values defined in [input/parameters/default.csv](https://github.com/tinosulzer/PyBaMM/blob/master/input/parameters/default.csv)"
    ]
   },
   {
@@ -78,7 +78,7 @@
    },
    "outputs": [],
    "source": [
-    "param = pybamm.BaseParameterValues()"
+    "param = pybamm.ParameterValues()"
    ]
   },
   {
diff --git a/pybamm/__init__.py b/pybamm/__init__.py
index 52442596..5bd18991 100644
--- a/pybamm/__init__.py
+++ b/pybamm/__init__.py
@@ -82,8 +82,9 @@ from .expression_tree.vector import Vector, StateVector
 #
 # Parameters class and methods
 #
-from .parameters.base_parameter_values import BaseParameterValues
+from .parameters.parameter_values import ParameterValues
 from .parameters import functions_lead_acid
+from .parameters import standard_parameters
 
 #
 # Mesh and Discretisation classes
diff --git a/pybamm/discretisations/base_mesh.py b/pybamm/discretisations/base_mesh.py
index f3278ccf..20607b33 100644
--- a/pybamm/discretisations/base_mesh.py
+++ b/pybamm/discretisations/base_mesh.py
@@ -22,7 +22,7 @@ class BaseMesh(dict):
 
     Parameters
     ----------
-    param : :class:`pybamm.BaseParameterValues.ParameterValues` instance
+    param : :class:`pybamm.ParameterValues.ParameterValues` instance
         The parameters defining the subdomain sizes.
     target_npts : int
         The target number of points in each domain. The mesh will be created
diff --git a/pybamm/models/electrolyte_current.py b/pybamm/models/electrolyte_current.py
index b01a8ea4..582133d4 100644
--- a/pybamm/models/electrolyte_current.py
+++ b/pybamm/models/electrolyte_current.py
@@ -47,7 +47,7 @@ class ElectrolyteCurrentModel(pybamm.BaseModel):
 
         Parameters
         ----------
-        param : :class:`pybamm.BaseParameterValues` instance
+        param : :class:`pybamm.ParameterValues` instance
             The parameters of the simulation
         operators : :class:`pybamm.Operators` instance
             The spatial operators.
diff --git a/pybamm/models/submodels/electrolyte.py b/pybamm/models/submodels/electrolyte.py
index a7e6cdcc..9eb26b65 100644
--- a/pybamm/models/submodels/electrolyte.py
+++ b/pybamm/models/submodels/electrolyte.py
@@ -16,7 +16,7 @@ class StefanMaxwellDiffusion(object):
 
         Parameters
         ----------
-        subparam : :class:`pybamm.BaseParameterValues.Parameters` subclass instance
+        subparam : :class:`pybamm.ParameterValues.Parameters` subclass instance
             The parameters of the simulation
         suboperators : :class:`pybamm.operators.Operators` subclass instance
             The spatial operators.
@@ -102,7 +102,7 @@ class StefanMaxwellDiffusion(object):
     #
     #     Parameters
     #     ----------
-    #     subparam : pybamm.BaseParameterValues.Parameter() instance
+    #     subparam : pybamm.ParameterValues.Parameter() instance
     #         The parameters of the simulation
     #     variables : 2-tuple (c, e) of array_like, shape (n,)
     #         The concentration, and potential difference.
diff --git a/pybamm/models/submodels/interface.py b/pybamm/models/submodels/interface.py
index 1c6ae070..23dc50fb 100644
--- a/pybamm/models/submodels/interface.py
+++ b/pybamm/models/submodels/interface.py
@@ -13,7 +13,7 @@ class Interface(object):
 
     Parameters
     ----------
-    subparam : :class:`pybamm.BaseParameterValues.Parameters` subclass instance
+    subparam : :class:`pybamm.ParameterValues.Parameters` subclass instance
         The parameters of the simulation for reactions in the electrode.
     submesh : :class:`pybamm.mesh.Mesh` subclass instance
         The mesh in the electrode.
diff --git a/pybamm/parameters/base_parameter_values.py b/pybamm/parameters/parameter_values.py
similarity index 99%
rename from pybamm/parameters/base_parameter_values.py
rename to pybamm/parameters/parameter_values.py
index d23fca0b..3d880f6e 100644
--- a/pybamm/parameters/base_parameter_values.py
+++ b/pybamm/parameters/parameter_values.py
@@ -9,7 +9,7 @@ import pandas as pd
 import copy
 
 
-class BaseParameterValues(dict):
+class ParameterValues(dict):
     """
     The parameter values for a simulation.
 
diff --git a/pybamm/parameters/standard_parameters.py b/pybamm/parameters/standard_parameters.py
new file mode 100644
index 00000000..82f7912e
--- /dev/null
+++ b/pybamm/parameters/standard_parameters.py
@@ -0,0 +1,32 @@
+#
+# Standard parameters for battery models
+#
+"""
+Standard Parameters for battery models
+
+Geometric
+---------
+
+Ln, Ls, Lp
+    The widths of the negative electrode, separator and positive electrode respectively
+L
+    The width of a single cell
+ln, ls, lp
+    The dimesionless widths of the negative electrode, separator and positive
+    electrode respectively
+
+"""
+from __future__ import absolute_import, division
+from __future__ import print_function, unicode_literals
+from pybamm import Parameter
+
+# Lengths
+Ln = Parameter("Ln")
+Ls = Parameter("Ls")
+Lp = Parameter("Lp")
+L = Ln + Ls + Lp
+
+# Dimensionless lengths
+ln = Ln / L
+ls = Ls / L
+lp = Lp / L
diff --git a/pybamm/simulation.py b/pybamm/simulation.py
index a9604f88..500f56d2 100644
--- a/pybamm/simulation.py
+++ b/pybamm/simulation.py
@@ -18,7 +18,7 @@ class Simulation(object):
     model : pybamm.models.(modelname).(ModelName)() instance
         The model to be used for the simulation. (modelname) and (ModelName)
         refer to a module and class to be chosen.
-    param : :class:`pybamm.BaseParameterValues.Parameters` instance
+    param : :class:`pybamm.ParameterValues.Parameters` instance
         The parameters to be used for the simulation.
     mesh : :class:`pybamm.mesh.Mesh` instance
         The mesh to be used for the simulation.
@@ -32,7 +32,7 @@ class Simulation(object):
     def __init__(self, model, param=None, mesh=None, solver=None, name="unnamed"):
         # Defaults
         if param is None:
-            param = pybamm.BaseParameterValues()
+            param = pybamm.ParameterValues()
         if mesh is None:
             mesh = pybamm.Mesh(param)
         if solver is None:


Test patch:
diff --git a/tests/test_discretisations/test_finite_volume_discretisations.py b/tests/test_discretisations/test_finite_volume_discretisations.py
index 834f1a55..eadabc9b 100644
--- a/tests/test_discretisations/test_finite_volume_discretisations.py
+++ b/tests/test_discretisations/test_finite_volume_discretisations.py
@@ -9,7 +9,7 @@ import unittest
 
 class TestFiniteVolumeDiscretisation(unittest.TestCase):
     def test_grad_div_shapes(self):
-        param = pybamm.BaseParameterValues(
+        param = pybamm.ParameterValues(
             base_parameters={"Ln": 0.1, "Ls": 0.2, "Lp": 0.3}
         )
         mesh = pybamm.FiniteVolumeMacroMesh(param, 2)
@@ -48,7 +48,7 @@ class TestFiniteVolumeDiscretisation(unittest.TestCase):
 
     def test_grad_convergence(self):
         # Convergence
-        param = pybamm.BaseParameterValues(
+        param = pybamm.ParameterValues(
             base_parameters={"Ln": 0.1, "Ls": 0.2, "Lp": 0.3}
         )
         var = pybamm.Variable("var", domain=["whole cell"])
@@ -82,7 +82,7 @@ class TestFiniteVolumeDiscretisation(unittest.TestCase):
     @unittest.skip("div errors do not converge as expected")
     def test_div_convergence(self):
         # Convergence
-        param = pybamm.BaseParameterValues(
+        param = pybamm.ParameterValues(
             base_parameters={"Ln": 0.1, "Ls": 0.2, "Lp": 0.3}
         )
         var = pybamm.Variable("var", domain=["whole cell"])
diff --git a/tests/test_discretisations/test_finite_volume_meshes.py b/tests/test_discretisations/test_finite_volume_meshes.py
index cd4e122b..983a83b4 100644
--- a/tests/test_discretisations/test_finite_volume_meshes.py
+++ b/tests/test_discretisations/test_finite_volume_meshes.py
@@ -9,7 +9,7 @@ import unittest
 
 class TestFiniteVolumeMesh(unittest.TestCase):
     def test_mesh_creation(self):
-        param = pybamm.BaseParameterValues(
+        param = pybamm.ParameterValues(
             base_parameters={"Ln": 0.1, "Ls": 0.2, "Lp": 0.3}
         )
         mesh = pybamm.FiniteVolumeMacroMesh(param, 50)
@@ -32,7 +32,7 @@ class TestFiniteVolumeMesh(unittest.TestCase):
         )
 
     def test_mesh_sizes(self):
-        param = pybamm.BaseParameterValues(
+        param = pybamm.ParameterValues(
             base_parameters={"Ln": 0.01, "Ls": 0.5, "Lp": 0.12}
         )
         mesh = pybamm.FiniteVolumeMacroMesh(param, 50)
diff --git a/tests/test_parameters/test_base_parameter_values.py b/tests/test_parameters/test_parameter_values.py
similarity index 88%
rename from tests/test_parameters/test_base_parameter_values.py
rename to tests/test_parameters/test_parameter_values.py
index 20320192..f951a025 100644
--- a/tests/test_parameters/test_base_parameter_values.py
+++ b/tests/test_parameters/test_parameter_values.py
@@ -7,24 +7,24 @@ import unittest
 import numpy as np
 
 
-class TestBaseModel(unittest.TestCase):
+class TestParameterValues(unittest.TestCase):
     def test_read_parameters_csv(self):
-        data = pybamm.BaseParameterValues().read_parameters_csv(
+        data = pybamm.ParameterValues().read_parameters_csv(
             "input/parameters/lead-acid/default.csv"
         )
         self.assertEqual(data["R"], 8.314)
 
     def test_init(self):
         # from dict
-        param = pybamm.BaseParameterValues({"a": 1})
+        param = pybamm.ParameterValues({"a": 1})
         self.assertEqual(param["a"], 1)
         # from file
-        param = pybamm.BaseParameterValues("input/parameters/lead-acid/default.csv")
+        param = pybamm.ParameterValues("input/parameters/lead-acid/default.csv")
         self.assertEqual(param["R"], 8.314)
 
     def test_overwrite(self):
         # from dicts
-        param = pybamm.BaseParameterValues(
+        param = pybamm.ParameterValues(
             base_parameters={"a": 1, "b": 2}, optional_parameters={"b": 3}
         )
         self.assertEqual(param["a"], 1)
@@ -32,7 +32,7 @@ class TestBaseModel(unittest.TestCase):
         param.update({"a": 4})
         self.assertEqual(param["a"], 4)
         # from files
-        param = pybamm.BaseParameterValues(
+        param = pybamm.ParameterValues(
             base_parameters="input/parameters/lead-acid/default.csv",
             optional_parameters="input/parameters/lead-acid/optional_test.csv",
         )
@@ -40,12 +40,12 @@ class TestBaseModel(unittest.TestCase):
         self.assertEqual(param["Ln"], 0.5)
 
     def test_get_parameter_value(self):
-        parameter_values = pybamm.BaseParameterValues({"a": 1})
+        parameter_values = pybamm.ParameterValues({"a": 1})
         param = pybamm.Parameter("a")
         self.assertEqual(parameter_values.get_parameter_value(param), 1)
 
     def test_process_symbol(self):
-        parameter_values = pybamm.BaseParameterValues({"a": 1, "b": 2})
+        parameter_values = pybamm.ParameterValues({"a": 1, "b": 2})
         # process parameter
         a = pybamm.Parameter("a")
         processed_a = parameter_values.process_symbol(a)
@@ -110,7 +110,7 @@ class TestBaseModel(unittest.TestCase):
         model.rhs = {c: a * pybamm.grad(c)}
         model.initial_conditions = {c: b}
         model.boundary_conditions = {}
-        parameter_values = pybamm.BaseParameterValues({"a": 1, "b": 2})
+        parameter_values = pybamm.ParameterValues({"a": 1, "b": 2})
         parameter_values.process(model)
         self.assertTrue(isinstance(model.rhs[c].children[0], pybamm.Scalar))
         self.assertEqual(model.rhs[c].children[0].value, 1)
diff --git a/tests/test_parameters/test_parameter_values_lead_acid.py b/tests/test_parameters/test_parameter_values_lead_acid.py
index 4e3dc63e..9eee1c34 100644
--- a/tests/test_parameters/test_parameter_values_lead_acid.py
+++ b/tests/test_parameters/test_parameter_values_lead_acid.py
@@ -15,24 +15,24 @@ class TestParameters(unittest.TestCase):
         self.assertEqual(data["R"], 8.314)
 
     def test_name_string_to_class_string(self):
-        ntc = pybamm.BaseParameterValues.parameters.name_string_to_class_string
+        ntc = pybamm.ParameterValues.parameters.name_string_to_class_string
         self.assertEqual(ntc("hello"), "_HelloParameters")
         self.assertEqual(ntc("hello_world"), "_HelloWorldParameters")
         self.assertEqual(ntc("helloworld"), "_HelloworldParameters")
 
     def test_parameters_init(self):
         with self.assertRaises(NotImplementedError):
-            pybamm.BaseParameterValues(tests="not a test")
+            pybamm.ParameterValues(tests="not a test")
         with self.assertRaises(NotImplementedError):
-            param = pybamm.BaseParameterValues(chemistry="not a chemistry")
+            param = pybamm.ParameterValues(chemistry="not a chemistry")
         for chemistry in ["lead-acid"]:  # pybamm.KNOWN_CHEMISTRIES:
-            param = pybamm.BaseParameterValues(chemistry=chemistry)
+            param = pybamm.ParameterValues(chemistry=chemistry)
             self.assertEqual(param._raw["R"], 8.314)
             self.assertEqual(param._func.D_eff(param, 1, 1), 1)
 
     def test_parameters_options(self):
         # test dictionary input
-        param = pybamm.BaseParameterValues(
+        param = pybamm.ParameterValues(
             chemistry="lead-acid",
             optional_parameters={"Ln": 1 / 3, "Ls": 0.25, "Lp": 0.25},
         )
@@ -43,7 +43,7 @@ class TestParameters(unittest.TestCase):
         )
 
         # Test file input
-        param = pybamm.BaseParameterValues(
+        param = pybamm.ParameterValues(
             chemistry="lead-acid", optional_parameters="optional_test.csv"
         )
         self.assertEqual(param._raw["Ln"], 0.5)
@@ -53,7 +53,7 @@ class TestParameters(unittest.TestCase):
         )
 
     def test_parameters_update_raw(self):
-        param = pybamm.BaseParameterValues(chemistry="lead-acid")
+        param = pybamm.ParameterValues(chemistry="lead-acid")
         param.update_raw({"Ln": 0.5})
         self.assertEqual(param._raw["Ln"], 0.5)
         self.assertAlmostEqual(
@@ -62,7 +62,7 @@ class TestParameters(unittest.TestCase):
 
     def test_parameters_defaults_lead_acid(self):
         # Tests on how the parameters interact
-        param = pybamm.BaseParameterValues(chemistry="lead-acid")
+        param = pybamm.ParameterValues(chemistry="lead-acid")
         mesh = pybamm.Mesh(param, 10)
         param.set_mesh(mesh)
 
@@ -88,7 +88,7 @@ class TestParameters(unittest.TestCase):
 
     def test_functions_lead_acid(self):
         # Tests on how the parameters interact
-        param = pybamm.BaseParameterValues(chemistry="lead-acid")
+        param = pybamm.ParameterValues(chemistry="lead-acid")
         mesh = pybamm.Mesh(param, 10)
         param.set_mesh(mesh)
         # Known values for dimensionless functions
@@ -105,7 +105,7 @@ class TestParameters(unittest.TestCase):
         self.assertGreater(param.pos_reactions.U(1), param.pos_reactions.U(0.5))
 
     def test_mesh_dependent_parameters(self):
-        param = pybamm.BaseParameterValues(chemistry="lead-acid")
+        param = pybamm.ParameterValues(chemistry="lead-acid")
         mesh = pybamm.Mesh(param, 10)
         param.set_mesh(mesh)
 
diff --git a/tests/test_parameters/test_standard_parameters.py b/tests/test_parameters/test_standard_parameters.py
new file mode 100644
index 00000000..b5d8cf5c
--- /dev/null
+++ b/tests/test_parameters/test_standard_parameters.py
@@ -0,0 +1,40 @@
+#
+# Tests for the standard parameters
+#
+import pybamm
+
+import unittest
+
+
+class TestStandardParameters(unittest.TestCase):
+    def test_geometric_parameters(self):
+        Ln = pybamm.standard_parameters.Ln
+        Ls = pybamm.standard_parameters.Ls
+        Lp = pybamm.standard_parameters.Lp
+        L = pybamm.standard_parameters.L
+        ln = pybamm.standard_parameters.ln
+        ls = pybamm.standard_parameters.ls
+        lp = pybamm.standard_parameters.lp
+
+        parameter_values = pybamm.ParameterValues(
+            base_parameters={"Ln": 0.05, "Ls": 0.02, "Lp": 0.21}
+        )
+        Ln_eval = parameter_values.process_symbol(Ln)
+        Ls_eval = parameter_values.process_symbol(Ls)
+        Lp_eval = parameter_values.process_symbol(Lp)
+        L_eval = parameter_values.process_symbol(L)
+        self.assertEqual((Ln_eval + Ls_eval + Lp_eval).evaluate(), L_eval.evaluate())
+        self.assertEqual((Ln_eval + Ls_eval + Lp_eval).id, L_eval.id)
+        ln_eval = parameter_values.process_symbol(ln)
+        ls_eval = parameter_values.process_symbol(ls)
+        lp_eval = parameter_values.process_symbol(lp)
+        self.assertAlmostEqual((ln_eval + ls_eval + lp_eval).evaluate(), 1)
+
+
+if __name__ == "__main__":
+    print("Add -v for more debug output")
+    import sys
+
+    if "-v" in sys.argv:
+        debug = True
+    unittest.main()