From a4fea21d44ce5a4428c05a43e14ba5a739a4bda2 Mon Sep 17 00:00:00 2001 From: Valentin Sulzer Date: Tue, 16 Aug 2022 17:06:52 +0100 Subject: [PATCH 1/4] simplify nondimensionalisation of kinetics --- .../full_battery_models/lithium_ion/basic_dfn.py | 4 ++-- .../lithium_ion/basic_dfn_half_cell.py | 2 +- .../full_battery_models/lithium_ion/basic_spm.py | 4 ++-- pybamm/models/submodels/interface/base_interface.py | 10 +++++----- pybamm/parameters/lithium_ion_parameters.py | 4 +--- .../test_parameters/test_lithium_ion_parameters.py | 9 --------- 6 files changed, 11 insertions(+), 22 deletions(-) diff --git a/pybamm/models/full_battery_models/lithium_ion/basic_dfn.py b/pybamm/models/full_battery_models/lithium_ion/basic_dfn.py index 6133df35fb..c83afe500e 100644 --- a/pybamm/models/full_battery_models/lithium_ion/basic_dfn.py +++ b/pybamm/models/full_battery_models/lithium_ion/basic_dfn.py @@ -123,7 +123,7 @@ def __init__(self, name="Doyle-Fuller-Newman model"): # right side. This is also accessible via `boundary_value(x, "right")`, with # "left" providing the boundary value of the left side c_s_surf_n = pybamm.surf(c_s_n) - j0_n = param.n.gamma * param.n.j0(c_e_n, c_s_surf_n, T) / param.n.C_r + j0_n = param.n.j0(c_e_n, c_s_surf_n, T) j_n = ( 2 * j0_n @@ -132,7 +132,7 @@ def __init__(self, name="Doyle-Fuller-Newman model"): ) ) c_s_surf_p = pybamm.surf(c_s_p) - j0_p = param.p.gamma * param.p.j0(c_e_p, c_s_surf_p, T) / param.p.C_r + j0_p = param.p.j0(c_e_p, c_s_surf_p, T) j_s = pybamm.PrimaryBroadcast(0, "separator") j_p = ( 2 diff --git a/pybamm/models/full_battery_models/lithium_ion/basic_dfn_half_cell.py b/pybamm/models/full_battery_models/lithium_ion/basic_dfn_half_cell.py index fe1dc113de..6a595b503b 100644 --- a/pybamm/models/full_battery_models/lithium_ion/basic_dfn_half_cell.py +++ b/pybamm/models/full_battery_models/lithium_ion/basic_dfn_half_cell.py @@ -124,7 +124,7 @@ def __init__(self, options=None, name="Doyle-Fuller-Newman half cell model"): b_e_w = param.p.b_e # Interfacial reactions - j0_w = param.p.j0(c_e_w, c_s_surf_w, T) / param.p.C_r + j0_w = param.p.j0(c_e_w, c_s_surf_w, T) U_w = param.p.U ne_w = param.p.ne diff --git a/pybamm/models/full_battery_models/lithium_ion/basic_spm.py b/pybamm/models/full_battery_models/lithium_ion/basic_spm.py index 727c7e2da2..b26605b5f7 100644 --- a/pybamm/models/full_battery_models/lithium_ion/basic_spm.py +++ b/pybamm/models/full_battery_models/lithium_ion/basic_spm.py @@ -139,8 +139,8 @@ def __init__(self, name="Single Particle Model"): # (Some) variables ###################### # Interfacial reactions - j0_n = param.n.gamma * param.n.j0(1, c_s_surf_n, T) / param.n.C_r - j0_p = param.p.gamma * param.p.j0(1, c_s_surf_p, T) / param.p.C_r + j0_n = param.n.j0(1, c_s_surf_n, T) + j0_p = param.p.j0(1, c_s_surf_p, T) eta_n = (2 / param.n.ne) * pybamm.arcsinh(j_n / (2 * j0_n)) eta_p = (2 / param.p.ne) * pybamm.arcsinh(j_p / (2 * j0_p)) phi_s_n = 0 diff --git a/pybamm/models/submodels/interface/base_interface.py b/pybamm/models/submodels/interface/base_interface.py index e27b58b2e4..fb88b4a51d 100644 --- a/pybamm/models/submodels/interface/base_interface.py +++ b/pybamm/models/submodels/interface/base_interface.py @@ -106,11 +106,11 @@ def _get_exchange_current_density(self, variables): c_e = c_e.orphans[0] T = T.orphans[0] - j0 = ( - domain_param.gamma - * domain_param.j0(c_e, c_s_surf, T) - / domain_param.C_r - ) + tol = 1e-8 + c_e = pybamm.maximum(tol, c_e) + c_s_surf = pybamm.maximum(tol, pybamm.minimum(c_s_surf, 1 - tol)) + + j0 = domain_param.j0(c_e, c_s_surf, T) elif self.reaction == "lithium metal plating": j0 = param.j0_plating(c_e, 1, T) diff --git a/pybamm/parameters/lithium_ion_parameters.py b/pybamm/parameters/lithium_ion_parameters.py index ec76573f09..40d1e2c899 100644 --- a/pybamm/parameters/lithium_ion_parameters.py +++ b/pybamm/parameters/lithium_ion_parameters.py @@ -887,9 +887,7 @@ def j0(self, c_e, c_s_surf, T): c_s_surf_dim = c_s_surf * self.c_max T_dim = self.main_param.Delta_T * T + self.main_param.T_ref - return ( - self.j0_dimensional(c_e_dim, c_s_surf_dim, T_dim) / self.j0_ref_dimensional - ) + return self.j0_dimensional(c_e_dim, c_s_surf_dim, T_dim) / self.j_scale def U(self, c_s, T): """Dimensionless open-circuit potential in the electrode""" diff --git a/tests/unit/test_parameters/test_lithium_ion_parameters.py b/tests/unit/test_parameters/test_lithium_ion_parameters.py index d3b6028c19..0070d680e2 100644 --- a/tests/unit/test_parameters/test_lithium_ion_parameters.py +++ b/tests/unit/test_parameters/test_lithium_ion_parameters.py @@ -66,10 +66,6 @@ def test_lithium_ion(self): 8, ) - np.testing.assert_almost_equal( - values.evaluate(param.n.gamma / param.n.C_r * c_rate), 26.6639, 3 - ) - # j0_p np.testing.assert_almost_equal( values.evaluate( @@ -79,11 +75,6 @@ def test_lithium_ion(self): 8, ) - # gamma_p / C_r_p - np.testing.assert_almost_equal( - values.evaluate(param.p.gamma / param.p.C_r * c_rate), 1.366, 3 - ) - # particle dynamics # neg diffusion coefficient np.testing.assert_almost_equal( From aa63cd7d57bbd98781b73ed59fd9abf4fbc312d1 Mon Sep 17 00:00:00 2001 From: Valentin Sulzer Date: Tue, 16 Aug 2022 17:27:25 +0100 Subject: [PATCH 2/4] remove tol accidentally added --- pybamm/models/submodels/interface/base_interface.py | 4 ---- 1 file changed, 4 deletions(-) diff --git a/pybamm/models/submodels/interface/base_interface.py b/pybamm/models/submodels/interface/base_interface.py index fb88b4a51d..ec4d128f1b 100644 --- a/pybamm/models/submodels/interface/base_interface.py +++ b/pybamm/models/submodels/interface/base_interface.py @@ -106,10 +106,6 @@ def _get_exchange_current_density(self, variables): c_e = c_e.orphans[0] T = T.orphans[0] - tol = 1e-8 - c_e = pybamm.maximum(tol, c_e) - c_s_surf = pybamm.maximum(tol, pybamm.minimum(c_s_surf, 1 - tol)) - j0 = domain_param.j0(c_e, c_s_surf, T) elif self.reaction == "lithium metal plating": From bcbd4996ec22033f07b3b08521a475510a5ac7e7 Mon Sep 17 00:00:00 2001 From: Valentin Sulzer Date: Wed, 17 Aug 2022 19:14:19 +0100 Subject: [PATCH 3/4] changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index dd1cd08704..4f6a34c2a0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,7 @@ ## Optimizations +- Simplified scaling for the exchange-current density. The dimensionless parameter `C_r` is kept, but no longer used anywhere ([#2238](https://github.com/pybamm-team/PyBaMM/pull/2238)) - Added limits for variables in some functions to avoid division by zero, sqrt(negative number), etc ([#2213](https://github.com/pybamm-team/PyBaMM/pull/2213)) # [v22.7](https://github.com/pybamm-team/PyBaMM/tree/v22.7) - 2022-07-31 From 3ea25a59abd1922f1b3fd036e0810c17c7f1233c Mon Sep 17 00:00:00 2001 From: Valentin Sulzer Date: Wed, 17 Aug 2022 22:48:18 +0100 Subject: [PATCH 4/4] fix coverage --- tests/unit/test_expression_tree/test_symbol.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/unit/test_expression_tree/test_symbol.py b/tests/unit/test_expression_tree/test_symbol.py index f31611f967..9ed32588a5 100644 --- a/tests/unit/test_expression_tree/test_symbol.py +++ b/tests/unit/test_expression_tree/test_symbol.py @@ -34,6 +34,8 @@ def check_are_equal(children1, children2): def test_symbol_domains(self): a = pybamm.Symbol("a", domain="test") self.assertEqual(a.domain, ["test"]) + # test for updating domain with same as existing domain + a.domains = {"primary": ["test"]} self.assertEqual(a.domains["primary"], ["test"]) a = pybamm.Symbol("a", domain=["t", "e", "s"]) self.assertEqual(a.domain, ["t", "e", "s"])