Skip to content

Commit

Permalink
Merge pull request #2236 from pybamm-team/simplify-reaction-scaling
Browse files Browse the repository at this point in the history
simplify nondimensionalisation of kinetics
  • Loading branch information
valentinsulzer authored Aug 22, 2022
2 parents a1bc678 + 6f56220 commit 1472d97
Show file tree
Hide file tree
Showing 8 changed files with 10 additions and 22 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,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
Expand Down
4 changes: 2 additions & 2 deletions pybamm/models/full_battery_models/lithium_ion/basic_dfn.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
4 changes: 2 additions & 2 deletions pybamm/models/full_battery_models/lithium_ion/basic_spm.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
6 changes: 1 addition & 5 deletions pybamm/models/submodels/interface/base_interface.py
Original file line number Diff line number Diff line change
Expand Up @@ -106,11 +106,7 @@ 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
)
j0 = domain_param.j0(c_e, c_s_surf, T)

elif self.reaction == "lithium metal plating":
j0 = param.j0_plating(c_e, 1, T)
Expand Down
4 changes: 1 addition & 3 deletions pybamm/parameters/lithium_ion_parameters.py
Original file line number Diff line number Diff line change
Expand Up @@ -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"""
Expand Down
2 changes: 2 additions & 0 deletions tests/unit/test_expression_tree/test_symbol.py
Original file line number Diff line number Diff line change
Expand Up @@ -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"])
Expand Down
9 changes: 0 additions & 9 deletions tests/unit/test_parameters/test_lithium_ion_parameters.py
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand All @@ -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(
Expand Down

0 comments on commit 1472d97

Please sign in to comment.