working on unit tests

CHANGELOG.md

@@ -12,11 +12,8 @@
 
 ## Breaking changes
 
-<<<<<<< HEAD
-
-- # All PyBaMM models are now dimensional. This has been benchmarked against dimensionless models and found to give around the same solve time. Implementing dimensional models greatly reduces the barrier to entry for adding new models. However, this comes with several breaking changes: (i) the `timescale` and `length_scales` attributes of a model have been removed (they are no longer needed) (ii) several dimensionless variables are no longer defined, but the corresponding dimensional variables can still be accessed by addin the units to the name (iii) some parameters used only for non-dimensionalization, such as "Typical current [A]", have been removed
+- All PyBaMM models are now dimensional. This has been benchmarked against dimensionless models and found to give around the same solve time. Implementing dimensional models greatly reduces the barrier to entry for adding new models. However, this comes with several breaking changes: (i) the `timescale` and `length_scales` attributes of a model have been removed (they are no longer needed) (ii) several dimensionless variables are no longer defined, but the corresponding dimensional variables can still be accessed by addin the units to the name (iii) some parameters used only for non-dimensionalization, such as "Typical current [A]", have been removed
 - Removed code for generating `ModelingToolkit` problems ([#2432](https://github.com/pybamm-team/PyBaMM/pull/2432))
-  > > > > > > > develop
 - Removed `FirstOrder` and `Composite` lead-acid models, and some submodels specific to those models ([#2431](https://github.com/pybamm-team/PyBaMM/pull/2431))
 
 # [v22.10](https://github.com/pybamm-team/PyBaMM/tree/v22.10) - 2022-10-31

examples/scripts/compare_lithium_ion.py

@@ -7,9 +7,8 @@
 # load models
 models = [
     pybamm.lithium_ion.SPM(),
-    pybamm.lithium_ion.SPM({"particle": "uniform profile"}),
-    # pybamm.lithium_ion.BasicDFN(),
-    # pybamm.lithium_ion.DFN(),
+    pybamm.lithium_ion.SPMe(),
+    pybamm.lithium_ion.DFN(),
     # pybamm.lithium_ion.NewmanTobias(),
 ]
 
@@ -18,17 +17,11 @@
 for model in models:
     sim = pybamm.Simulation(
         model,
-        # parameter_values=pybamm.ParameterValues("Ai2020"),
-        solver=pybamm.CasadiSolver(),  # root_method="lm"),
+        # parameter_values=pybamm.ParameterValues("Ecker2015"),
+        solver=pybamm.CasadiSolver(dt_max=600),  # root_method="lm"),
     )
     sim.solve([0, 3600])
     sims.append(sim)
 
 # plot
-pybamm.dynamic_plot(
-    sims,
-    [
-        "Average negative particle concentration [mol.m-3]",
-        "Average positive particle concentration [mol.m-3]",
-    ],
-)
+pybamm.dynamic_plot(sims)

examples/scripts/compare_particle_models.py

@@ -5,16 +5,16 @@
 
 # load models
 models = [
-    pybamm.lithium_ion.SPM(
+    pybamm.lithium_ion.DFN(
         options={"particle": "Fickian diffusion"}, name="Fickian diffusion"
     ),
-    pybamm.lithium_ion.SPM(
+    pybamm.lithium_ion.DFN(
         options={"particle": "uniform profile"}, name="uniform profile"
     ),
-    pybamm.lithium_ion.SPM(
+    pybamm.lithium_ion.DFN(
         options={"particle": "quadratic profile"}, name="quadratic profile"
     ),
-    pybamm.lithium_ion.SPM(
+    pybamm.lithium_ion.DFN(
         options={"particle": "quartic profile"}, name="quartic profile"
     ),
 ]

pybamm/expression_tree/concatenations.py

@@ -382,11 +382,8 @@ def __init__(self, *children):
                 raise ValueError("Cannot concatenate symbols with different references")
 
         super().__init__(*children, name=name)
-        # Overly tight bounds, can edit later if required
-        self.bounds = (
-            np.max([child.bounds[0] for child in children]),
-            np.min([child.bounds[1] for child in children]),
-        )
+        # 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/models/full_battery_models/lithium_ion/basic_dfn_half_cell.py

@@ -139,7 +139,7 @@ def __init__(self, options=None, name="Doyle-Fuller-Newman half cell model"):
             * j0_w
             * pybamm.sinh(ne_w / 2 * F_RT * (phi_s_w - phi_e_w - U_w(sto_surf_w, T)))
         )
-        R_w = param.p.prim.R
+        R_w = param.p.prim.R_typ
         a_w = 3 * eps_s_w / R_w
         a_j_w = a_w * j_w
         a_j_s = pybamm.PrimaryBroadcast(0, "separator")

pybamm/models/submodels/convection/through_cell/explicit_convection.py

@@ -33,15 +33,15 @@ def get_coupled_variables(self, variables):
             ]
             if domain == "negative electrode":
                 x_n = pybamm.standard_spatial_vars.x_n
-                beta_k = param.n.beta
-                p_k = beta_k * j_k_av * (-(x_n**2) + param.n.l**2) / 2 + p_s
-                v_box_k = beta_k * j_k_av * x_n
+                DeltaV_k = param.n.DeltaV
+                p_k = DeltaV_k * j_k_av * (-(x_n**2) + param.n.L**2) / 2 + p_s
+                v_box_k = DeltaV_k * j_k_av * x_n
             elif domain == "positive electrode":
                 x_p = pybamm.standard_spatial_vars.x_p
-                beta_k = param.p.beta
-                p_k = beta_k * j_k_av * ((x_p - 1) ** 2 - param.p.l**2) / 2 + p_s
-                v_box_k = beta_k * j_k_av * (x_p - 1)
-            div_v_box_k = pybamm.PrimaryBroadcast(beta_k * j_k_av, domain)
+                DeltaV_k = param.p.DeltaV
+                p_k = DeltaV_k * j_k_av * ((x_p - 1) ** 2 - param.p.L**2) / 2 + p_s
+                v_box_k = DeltaV_k * j_k_av * (x_p - 1)
+            div_v_box_k = pybamm.PrimaryBroadcast(DeltaV_k * j_k_av, domain)
 
             variables.update(
                 self._get_standard_convection_variables(
@@ -54,13 +54,13 @@ def get_coupled_variables(self, variables):
             "X-averaged separator transverse volume-averaged acceleration [m.s-2]"
         ]
         i_boundary_cc = variables["Current collector current density [A.m-2]"]
-        v_box_n_right = param.n.beta * i_boundary_cc
+        v_box_n_right = param.n.DeltaV * i_boundary_cc
         div_v_box_s_av = -div_Vbox_s
         div_v_box_s = pybamm.PrimaryBroadcast(div_v_box_s_av, "separator")
 
         # Simple formula for velocity in the separator
         x_s = pybamm.standard_spatial_vars.x_s
-        v_box_s = div_v_box_s_av * (x_s - param.n.l) + v_box_n_right
+        v_box_s = div_v_box_s_av * (x_s - param.n.L) + v_box_n_right
 
         variables.update(
             self._get_standard_sep_velocity_variables(v_box_s, div_v_box_s)

pybamm/models/submodels/convection/through_cell/full_convection.py

@@ -27,7 +27,7 @@ def get_fundamental_variables(self):
                 Domain = domain.capitalize()
                 # Electrolyte pressure
                 p_k = pybamm.Variable(
-                    f"{Domain} pressure",
+                    f"{Domain} pressure [Pa]",
                     domain=domain,
                     auxiliary_domains={"secondary": "current collector"},
                 )
@@ -49,20 +49,20 @@ def get_coupled_variables(self, variables):
 
         # Set up
         param = self.param
-        l_n = param.n.l
+        L_n = param.n.L
         x_s = pybamm.standard_spatial_vars.x_s
 
         # Transverse velocity in the separator determines through-cell velocity
         div_Vbox_s = variables[
-            "X-averaged separator transverse volume-averaged acceleration"
+            "X-averaged separator transverse volume-averaged acceleration [m.s-2]"
         ]
         i_boundary_cc = variables["Current collector current density [A.m-2]"]
-        v_box_n_right = param.n.beta * i_boundary_cc
+        v_box_n_right = param.n.DeltaV * i_boundary_cc
         div_v_box_s_av = -div_Vbox_s
         div_v_box_s = pybamm.PrimaryBroadcast(div_v_box_s_av, "separator")
 
         # Simple formula for velocity in the separator
-        v_box_s = div_v_box_s_av * (x_s - l_n) + v_box_n_right
+        v_box_s = div_v_box_s_av * (x_s - L_n) + v_box_n_right
 
         variables.update(
             self._get_standard_sep_velocity_variables(v_box_s, div_v_box_s)
@@ -76,8 +76,8 @@ def get_coupled_variables(self, variables):
         return variables
 
     def set_algebraic(self, variables):
-        p_n = variables["Negative electrode pressure"]
-        p_p = variables["Positive electrode pressure"]
+        p_n = variables["Negative electrode pressure [Pa]"]
+        p_p = variables["Positive electrode pressure [Pa]"]
 
         j_n = variables["Negative electrode interfacial current density [A.m-2]"]
         j_p = variables["Positive electrode interfacial current density [A.m-2]"]
@@ -87,14 +87,14 @@ def set_algebraic(self, variables):
 
         # Problems in the x-direction for p_n and p_p
         self.algebraic = {
-            p_n: pybamm.div(v_box_n) - self.param.n.beta * j_n,
-            p_p: pybamm.div(v_box_p) - self.param.p.beta * j_p,
+            p_n: pybamm.div(v_box_n) - self.param.n.DeltaV * j_n,
+            p_p: pybamm.div(v_box_p) - self.param.p.DeltaV * j_p,
         }
 
     def set_boundary_conditions(self, variables):
-        p_n = variables["Negative electrode pressure"]
-        p_s = variables["X-averaged separator pressure"]
-        p_p = variables["Positive electrode pressure"]
+        p_n = variables["Negative electrode pressure [Pa]"]
+        p_s = variables["X-averaged separator pressure [Pa]"]
+        p_p = variables["Positive electrode pressure [Pa]"]
 
         # Boundary conditions in x-direction for p_n and p_p
         self.boundary_conditions = {
@@ -103,7 +103,7 @@ def set_boundary_conditions(self, variables):
         }
 
     def set_initial_conditions(self, variables):
-        p_n = variables["Negative electrode pressure"]
-        p_p = variables["Positive electrode pressure"]
+        p_n = variables["Negative electrode pressure [Pa]"]
+        p_p = variables["Positive electrode pressure [Pa]"]
 
         self.initial_conditions = {p_n: pybamm.Scalar(0), p_p: pybamm.Scalar(0)}

pybamm/models/submodels/convection/transverse/full_convection.py

@@ -23,7 +23,7 @@ def __init__(self, param):
     def get_fundamental_variables(self):
 
         p_s = pybamm.Variable(
-            "X-averaged separator pressure", domain="current collector"
+            "X-averaged separator pressure [Pa]", domain="current collector"
         )
         variables = self._get_standard_separator_pressure_variables(p_s)
 
@@ -43,13 +43,13 @@ def get_fundamental_variables(self):
     def set_algebraic(self, variables):
         param = self.param
 
-        p_s = variables["X-averaged separator pressure"]
+        p_s = variables["X-averaged separator pressure [Pa]"]
         # Difference in negative and positive electrode velocities determines the
         # velocity in the separator
         i_boundary_cc = variables["Current collector current density [A.m-2]"]
-        v_box_n_right = param.n.beta * i_boundary_cc
-        v_box_p_left = param.p.beta * i_boundary_cc
-        d_vbox_s_dx = (v_box_p_left - v_box_n_right) / param.s.l
+        v_box_n_right = param.n.DeltaV * i_boundary_cc
+        v_box_p_left = param.p.DeltaV * i_boundary_cc
+        d_vbox_s_dx = (v_box_p_left - v_box_n_right) / param.s.L
 
         # Simple formula for velocity in the separator
         div_Vbox_s = -d_vbox_s_dx
@@ -61,7 +61,7 @@ def set_algebraic(self, variables):
         self.algebraic = {p_s: pybamm.div(Vbox_s) - div_Vbox_s}
 
     def set_boundary_conditions(self, variables):
-        p_s = variables["X-averaged separator pressure"]
+        p_s = variables["X-averaged separator pressure [Pa]"]
 
         # Boundary conditions in z-direction for p_s (left=bottom, right=top)
         self.boundary_conditions = {
@@ -72,6 +72,6 @@ def set_boundary_conditions(self, variables):
         }
 
     def set_initial_conditions(self, variables):
-        p_s = variables["X-averaged separator pressure"]
+        p_s = variables["X-averaged separator pressure [Pa]"]
 
         self.initial_conditions = {p_s: pybamm.Scalar(0)}

pybamm/models/submodels/convection/transverse/uniform_convection.py

@@ -37,9 +37,9 @@ def get_coupled_variables(self, variables):
         # Difference in negative and positive electrode velocities determines the
         # velocity in the separator
         i_boundary_cc = variables["Current collector current density [A.m-2]"]
-        v_box_n_right = param.n.beta * i_boundary_cc
-        v_box_p_left = param.p.beta * i_boundary_cc
-        d_vbox_s_dx = (v_box_p_left - v_box_n_right) / param.s.l
+        v_box_n_right = param.n.DeltaV * i_boundary_cc
+        v_box_p_left = param.p.DeltaV * i_boundary_cc
+        d_vbox_s_dx = (v_box_p_left - v_box_n_right) / param.s.L
 
         # Simple formula for velocity in the separator
         div_Vbox_s = -d_vbox_s_dx

pybamm/models/submodels/electrode/ohm/full_ohm.py

@@ -25,20 +25,19 @@ def __init__(self, param, domain, options=None):
 
     def get_fundamental_variables(self):
         domain, Domain = self.domain_Domain
-        phi_s_var = pybamm.Variable(
-            f"{Domain} electrode potential variable [V]",
-            domain=f"{domain} electrode",
-            auxiliary_domains={"secondary": "current collector"},
-        )
-        variables = {f"{Domain} electrode potential variable [V]": phi_s_var}
-
         # potential is scaled with reference potential
         if domain == "negative":
-            phi_s = phi_s_var
+            reference = 0
         else:
-            phi_s = self.param.ocv_init + phi_s_var
+            reference = self.param.ocv_init
 
-        variables.update(self._get_standard_potential_variables(phi_s))
+        phi_s = pybamm.Variable(
+            f"{Domain} electrode potential [V]",
+            domain=f"{domain} electrode",
+            auxiliary_domains={"secondary": "current collector"},
+            reference=reference,
+        )
+        variables = self._get_standard_potential_variables(phi_s)
 
         return variables
 
@@ -66,7 +65,7 @@ def get_coupled_variables(self, variables):
     def set_algebraic(self, variables):
         domain, Domain = self.domain_Domain
 
-        phi_s = variables[f"{Domain} electrode potential variable [V]"]
+        phi_s = variables[f"{Domain} electrode potential [V]"]
         i_s = variables[f"{Domain} electrode current density [A.m-2]"]
 
         # Variable summing all of the interfacial current densities
@@ -103,6 +102,11 @@ def set_boundary_conditions(self, variables):
     def set_initial_conditions(self, variables):
         Domain = self.domain.capitalize()
 
-        phi_s = variables[f"{Domain} electrode potential variable [V]"]
+        phi_s = variables[f"{Domain} electrode potential [V]"]
+
+        if self.domain == "negative":
+            phi_s_init = pybamm.Scalar(0)
+        else:
+            phi_s_init = self.param.ocv_init
 
-        self.initial_conditions[phi_s] = pybamm.Scalar(0)
+        self.initial_conditions[phi_s] = phi_s_init

pybamm/models/submodels/electrolyte_conductivity/full_conductivity.py

@@ -26,24 +26,21 @@ def __init__(self, param, options=None):
 
     def get_fundamental_variables(self):
         phi_e_dict = {}
-        phi_e_var_dict = {}
         variables = {}
         for domain in self.options.whole_cell_domains:
             Dom = domain.capitalize().split()[0]
-            name = f"{Dom} electrolyte potential variable [V]"
-            phi_e_k_var = pybamm.Variable(
+            name = f"{Dom} electrolyte potential [V]"
+            phi_e_k = pybamm.Variable(
                 name,
                 domain=domain,
                 auxiliary_domains={"secondary": "current collector"},
+                reference=-self.param.n.prim.U_init,
             )
-            variables[name] = phi_e_k_var
-            phi_e_var_dict[domain] = phi_e_k_var
-            phi_e_k = -self.param.n.prim.U_init + phi_e_k_var
             phi_e_k.print_name = f"phi_e_{domain[0]}"
             phi_e_dict[domain] = phi_e_k
 
-        variables["Electrolyte potential variable [V]"] = pybamm.concatenation(
-            *phi_e_var_dict.values()
+        variables["Electrolyte potential [V]"] = pybamm.concatenation(
+            *phi_e_dict.values()
         )
 
         variables.update(self._get_standard_potential_variables(phi_e_dict))
@@ -70,7 +67,7 @@ def get_coupled_variables(self, variables):
         return variables
 
     def set_algebraic(self, variables):
-        phi_e = variables["Electrolyte potential variable [V]"]
+        phi_e = variables["Electrolyte potential [V]"]
         i_e = variables["Electrolyte current density [A.m-2]"]
 
         # Variable summing all of the interfacial current densities
@@ -82,5 +79,5 @@ def set_algebraic(self, variables):
         self.algebraic = {phi_e: pybamm.div(i_e) - sum_a_j}
 
     def set_initial_conditions(self, variables):
-        phi_e = variables["Electrolyte potential variable [V]"]
+        phi_e = variables["Electrolyte potential [V]"]
         self.initial_conditions = {phi_e: 0}

pybamm/models/submodels/electrolyte_diffusion/full_diffusion.py

@@ -33,6 +33,7 @@ def get_fundamental_variables(self):
                 domain=domain,
                 auxiliary_domains={"secondary": "current collector"},
                 bounds=(0, np.inf),
+                scale=self.param.c_e_typ,
             )
             eps_c_e_k.print_name = f"eps_c_e_{domain[0]}"
             eps_c_e_dict[domain] = eps_c_e_k

pybamm/models/submodels/interface/kinetics/diffusion_limited.py

@@ -86,8 +86,7 @@ def _get_diffusion_limited_current_density(self, variables):
                     * param.D_ox
                     * pybamm.BoundaryGradient(c_ox_s, "left")
                 )
-                N_ox_neg_sep_interface.domains = {"primary": "current collector"}
 
-                j = -N_ox_neg_sep_interface / param.C_e / -param.s_ox_Ox / param.n.L
+                j = -N_ox_neg_sep_interface / -param.s_ox_Ox / param.n.L
 
         return j

pybamm/models/submodels/interface/open_circuit_potential/single_ocp.py

@@ -50,7 +50,7 @@ def get_coupled_variables(self, variables):
             dUdT = pybamm.Scalar(0)
 
         elif self.reaction == "lead-acid oxygen":
-            ocp = self.phase_param.U_Ox
+            ocp = self.param.U_Ox
             dUdT = pybamm.Scalar(0)
 
         variables.update(self._get_standard_ocp_variables(ocp, dUdT))