#2418 tests

examples/scripts/compare_lithium_ion.py

@@ -9,16 +9,17 @@
     pybamm.lithium_ion.SPM(),
     pybamm.lithium_ion.SPMe(),
     pybamm.lithium_ion.DFN(),
-    pybamm.lithium_ion.NewmanTobias(),
+    # pybamm.lithium_ion.NewmanTobias(),
 ]
 
 # create and run simulations
 sims = []
+# parameter_values = pybamm.ParameterValues("Ecker2015")
 for model in models:
     sim = pybamm.Simulation(
         model,
-        # parameter_values=pybamm.ParameterValues("Ecker2015"),
-        solver=pybamm.CasadiSolver(dt_max=600),  # , root_method="lm"),
+        parameter_values=pybamm.ParameterValues("Ecker2015"),
+        solver=pybamm.CasadiSolver("fast"),  # dt_max=600),  # , root_method="lm"),
     )
     sim.solve([0, 3600])
     sims.append(sim)

examples/scripts/compare_particle_models.py

@@ -11,12 +11,12 @@
     pybamm.lithium_ion.DFN(
         options={"particle": "uniform profile"}, name="uniform profile"
     ),
-    pybamm.lithium_ion.DFN(
-        options={"particle": "quadratic profile"}, name="quadratic profile"
-    ),
-    pybamm.lithium_ion.DFN(
-        options={"particle": "quartic profile"}, name="quartic profile"
-    ),
+    # pybamm.lithium_ion.DFN(
+    #     options={"particle": "quadratic profile"}, name="quadratic profile"
+    # ),
+    # pybamm.lithium_ion.DFN(
+    #     options={"particle": "quartic profile"}, name="quartic profile"
+    # ),
 ]
 
 # pick parameter values
@@ -26,7 +26,8 @@
 sims = []
 for model in models:
     sim = pybamm.Simulation(model, parameter_values=parameter_values)
-    sim.solve([0, 3600])
+    solver = pybamm.CasadiSolver(root_method="lm")
+    sim.solve([0, 3600], solver=solver)
     sims.append(sim)
     print("Particle model: {}".format(model.name))
     print("Solve time: {}s".format(sim.solution.solve_time))

pybamm/expression_tree/concatenations.py

@@ -380,10 +380,13 @@ def __init__(self, *children):
                 self._reference = children[0].reference
             else:
                 raise ValueError("Cannot concatenate symbols with different references")
-
+            if all(
+                child.bounds[0] == children[0].bounds[0] for child in children
+            ) and all(child.bounds[1] == children[0].bounds[1] for child in children):
+                self.bounds = children[0].bounds
+            else:
+                raise ValueError("Cannot concatenate symbols with different bounds")
         super().__init__(*children, name=name)
-        # assume all children have the same bounds
-        self.bounds = children[0].bounds
 
         if not any(c._raw_print_name is None for c in children):
             print_name = intersect(

pybamm/expression_tree/variable.py

@@ -80,7 +80,14 @@ def __init__(
                     "Invalid bounds {}. ".format(bounds)
                     + "Lower bound should be strictly less than upper bound."
                 )
+
+        bounds = list(bounds)
+        for idx, bound in enumerate(bounds):
+            if isinstance(bound, numbers.Number):
+                bounds[idx] = pybamm.Scalar(bound)
+        bounds = tuple(bounds)
         self.bounds = bounds
+
         self.print_name = print_name
 
     def set_id(self):

pybamm/geometry/battery_geometry.py

@@ -90,7 +90,7 @@ def battery_geometry(
             geometry["current collector"] = {"z": {"position": 1}}
         elif current_collector_dimension == 1:
             geometry["current collector"] = {
-                "z": {"min": 0, "max": 1},
+                "z": {"min": 0, "max": geo.L_z},
                 "tabs": {
                     "negative": {"z_centre": geo.n.centre_z_tab},
                     "positive": {"z_centre": geo.p.centre_z_tab},

pybamm/models/base_model.py

@@ -291,26 +291,27 @@ def options(self, options):
 
     @property
     def timescale(self):
-        """Timescale of model, to be used for non-dimensionalising time when solving"""
-        return self._timescale
+        raise NotImplementedError(
+            "timescale has been removed since models are now dimensional"
+        )
 
     @timescale.setter
     def timescale(self, value):
-        """Set the timescale"""
-        self._timescale = value
+        raise NotImplementedError(
+            "timescale has been removed since models are now dimensional"
+        )
 
     @property
     def length_scales(self):
-        "Length scales of model"
-        return self._length_scales
+        raise NotImplementedError(
+            "length_scales has been removed since models are now dimensional"
+        )
 
     @length_scales.setter
     def length_scales(self, values):
-        "Set the length scale, converting any numbers to pybamm.Scalar"
-        for domain, scale in values.items():
-            if isinstance(scale, numbers.Number):
-                values[domain] = pybamm.Scalar(scale)
-        self._length_scales = values
+        raise NotImplementedError(
+            "length_scales has been removed since models are now dimensional"
+        )
 
     @property
     def default_var_pts(self):

pybamm/parameters/lead_acid_parameters.py

@@ -258,12 +258,19 @@ def _set_parameters(self):
         # thickness)
         self.L_cc = self.L
 
+        # Thermal
+        self.rho_c_p = self.therm.rho_c_p
+        self.lambda_ = self.therm.lambda_
+
         if self.domain == "separator":
             self.eps_max = pybamm.Parameter("Maximum porosity of separator")
             self.epsilon_init = self.eps_max
             self.b_e = self.geo.b_e
             self.epsilon_inactive = pybamm.Scalar(0)
             return
+        # for lead-acid the electrodes and current collector are the same
+        self.rho_c_p_cc = self.therm.rho_c_p
+        self.lambda_cc = self.therm.lambda_
 
         # Microstructure
         self.b_e = self.geo.b_e

pybamm/parameters/lithium_ion_parameters.py

@@ -523,10 +523,13 @@ def _set_parameters(self):
         if main.options["particle shape"] == "spherical":
             self.a_typ = 3 * pybamm.xyz_average(self.epsilon_s) / self.R_typ
 
-    def D(self, sto, T):
+    def D(self, c_s, T):
         """Dimensional diffusivity in particle. Note this is defined as a
         function of stochiometry"""
         Domain = self.domain.capitalize()
+        sto = c_s / self.c_max
+        tol = pybamm.settings.tolerances["D__c_s"]
+        sto = pybamm.maximum(pybamm.minimum(sto, 1 - tol), tol)
         inputs = {
             f"{self.phase_prefactor}{Domain} particle stoichiometry": sto,
             "Temperature [K]": T,

pybamm/parameters/parameter_values.py

@@ -491,8 +491,6 @@ def process_geometry(self, geometry):
         """
 
         def process_and_check(sym):
-            if isinstance(sym, numbers.Number):
-                return pybamm.Scalar(sym)
             new_sym = self.process_symbol(sym)
             if not isinstance(new_sym, pybamm.Scalar):
                 raise ValueError(

pybamm/settings.py

@@ -16,7 +16,8 @@ class Settings(object):
         "D_e__c_e": 10,  # dimensional
         "kappa_e__c_e": 10,  # dimensional
         "chi__c_e": 1e-2,  # dimensionless
-        "U__c_s": 1e-10,  # dimensional
+        "D__c_s": 1e-10,  # dimensionless
+        "U__c_s": 1e-10,  # dimensionless
         "j0__c_e": 1e-8,  # dimensionless
         "j0__c_s": 1e-8,  # dimensionless
         "macinnes__c_e": 1e-15,  # dimensionless

tests/unit/test_discretisations/test_discretisation.py

@@ -1136,13 +1136,13 @@ def test_concatenation_2D(self):
             "a",
             domain=["negative electrode"],
             auxiliary_domains={"secondary": "current collector"},
-            bounds=(-5, -2),
+            bounds=(0, 1),
         )
         b = pybamm.Variable(
             "b",
             domain=["separator"],
             auxiliary_domains={"secondary": "current collector"},
-            bounds=(6, 10),
+            bounds=(0, 1),
         )
         c = pybamm.Variable(
             "c",
@@ -1163,8 +1163,6 @@ def test_concatenation_2D(self):
         self.assertEqual(
             disc.y_slices[c], [slice(65, 100), slice(165, 200), slice(265, 300)]
         )
-        np.testing.assert_array_equal(disc.bounds[0], 6)
-        np.testing.assert_array_equal(disc.bounds[1], -2)
         expr = disc.process_symbol(conc)
         self.assertIsInstance(expr, pybamm.StateVector)
 

tests/unit/test_expression_tree/test_concatenations.py

@@ -125,6 +125,16 @@ def test_concatenations_scale(self):
         conc = pybamm.concatenation(a, b)
         self.assertEqual(conc.reference, 3)
 
+        a.bounds = (0, 1)
+        with self.assertRaisesRegex(
+            ValueError, "Cannot concatenate symbols with different bounds"
+        ):
+            pybamm.concatenation(a, b)
+
+        b.bounds = (0, 1)
+        conc = pybamm.concatenation(a, b)
+        self.assertEqual(conc.bounds, (0, 1))
+
     def test_concatenation_simplify(self):
         # Primary broadcast
         var = pybamm.Variable("var", "current collector")

tests/unit/test_parameters/test_base_parameters.py

@@ -9,18 +9,15 @@ class TestBaseParameters(unittest.TestCase):
     def test_getattr__(self):
         param = pybamm.LithiumIonParameters()
         # ending in _n / _s / _p
-        with self.assertRaisesRegex(AttributeError, "param.n.l"):
-            getattr(param, "l_n")
-        with self.assertRaisesRegex(AttributeError, "param.s.l"):
-            getattr(param, "l_s")
-        with self.assertRaisesRegex(AttributeError, "param.p.l"):
-            getattr(param, "l_p")
+        with self.assertRaisesRegex(AttributeError, "param.n.L"):
+            getattr(param, "L_n")
+        with self.assertRaisesRegex(AttributeError, "param.s.L"):
+            getattr(param, "L_s")
+        with self.assertRaisesRegex(AttributeError, "param.p.L"):
+            getattr(param, "L_p")
         # _n_ in the name
         with self.assertRaisesRegex(AttributeError, "param.n.prim.c_max"):
             getattr(param, "c_n_max")
-        # _p_ in the name, function
-        with self.assertRaisesRegex(AttributeError, "param.p.prim.U_dimensional"):
-            getattr(param, "U_p_dimensional")
 
         # _n_ or _p_ not in name
         with self.assertRaisesRegex(
@@ -32,9 +29,6 @@ def test_getattr__(self):
             getattr(pybamm.electrical_parameters, "c_s_test")
 
     def test__setattr__(self):
-        param = pybamm.ElectricalParameters()
-        self.assertEqual(param.I_typ.print_name, r"I{}^{typ}")
-
         # domain gets added as a subscript
         param = pybamm.GeometricParameters()
         self.assertEqual(param.n.L.print_name, r"L_n")

tests/unit/test_parameters/test_current_functions.py

@@ -19,14 +19,14 @@ def test_constant_current(self):
     def test_get_current_data(self):
         # test process parameters
         param = pybamm.electrical_parameters
-        dimensional_current = param.dimensional_current_with_time
+        current = param.current_with_time
         parameter_values = pybamm.ParameterValues(
             {"Current function [A]": "[current data]car_current"}
         )
-        dimensional_current_eval = parameter_values.process_symbol(dimensional_current)
+        current_eval = parameter_values.process_symbol(current)
 
         def current(t):
-            return dimensional_current_eval.evaluate(t=t)
+            return current_eval.evaluate(t=t)
 
         standard_tests = StandardCurrentFunctionTests([current], always_array=True)
         standard_tests.test_all()
@@ -38,7 +38,7 @@ def my_fun(t, A, omega):
 
         # choose amplitude and frequency
         param = pybamm.electrical_parameters
-        A = param.I_typ
+        A = 5
         omega = pybamm.Parameter("omega")
 
         def current(t):
@@ -51,11 +51,11 @@ def current(t):
                 "Current function [A]": current,
             }
         )
-        dimensional_current = param.dimensional_current_with_time
-        dimensional_current_eval = parameter_values.process_symbol(dimensional_current)
+        current = param.current_with_time
+        current_eval = parameter_values.process_symbol(current)
 
         def user_current(t):
-            return dimensional_current_eval.evaluate(t=t)
+            return current_eval.evaluate(t=t)
 
         # check output types
         standard_tests = StandardCurrentFunctionTests([user_current])
@@ -64,7 +64,7 @@ def user_current(t):
         # check output correct value
         time = np.linspace(0, 3600, 600)
         np.testing.assert_array_almost_equal(
-            user_current(time), 2 * np.sin(2 * np.pi * 3 * time)
+            user_current(time), 5 * np.sin(2 * np.pi * 3 * time)
         )