From 814e94d3d966e03820492542de0a2f68c305433d Mon Sep 17 00:00:00 2001 From: Ferran Brosa Planella Date: Thu, 3 Nov 2022 11:20:47 +0000 Subject: [PATCH 1/9] #2338 introduce alpha_SEI --- .../models/submodels/interface/sei/sei_growth.py | 15 +++++++++++---- pybamm/parameters/lithium_ion_parameters.py | 3 +++ 2 files changed, 14 insertions(+), 4 deletions(-) diff --git a/pybamm/models/submodels/interface/sei/sei_growth.py b/pybamm/models/submodels/interface/sei/sei_growth.py index dcd991919d..e82ffe5d94 100644 --- a/pybamm/models/submodels/interface/sei/sei_growth.py +++ b/pybamm/models/submodels/interface/sei/sei_growth.py @@ -100,9 +100,16 @@ def get_coupled_variables(self, variables): # Thermal prefactor for reaction, interstitial and EC models prefactor = 1 / (1 + self.param.Theta * T) - if self.options["SEI"] == "reaction limited": + # Define alpha_SEI depending on whether it is symmetric or asymmetric. This + # applies to "reaction limited" and "EC reaction limited" + if self.options["SEI"].endswith("(asymmetric)"): + alpha_SEI = phase_param.alpha_SEI + else: + alpha_SEI = 0.5 + + if self.options["SEI"].startswith("reaction limited"): C_sei = phase_param.C_sei_reaction - j_sei = -(1 / C_sei) * pybamm.exp(-0.5 * prefactor * eta_SEI) + j_sei = -(1 / C_sei) * pybamm.exp(-alpha_SEI * prefactor * eta_SEI) elif self.options["SEI"] == "electron-migration limited": U_inner = phase_param.U_inner_electron @@ -117,7 +124,7 @@ def get_coupled_variables(self, variables): C_sei = phase_param.C_sei_solvent j_sei = -1 / (C_sei * L_sei_outer) - elif self.options["SEI"] == "ec reaction limited": + elif self.options["SEI"].startswith("ec reaction limited"): C_sei_ec = phase_param.C_sei_ec C_ec = phase_param.C_ec @@ -129,7 +136,7 @@ def get_coupled_variables(self, variables): # so # j_sei = -C_sei_ec * exp() / (1 + L_sei * C_ec * C_sei_ec * exp()) # c_ec = 1 / (1 + L_sei * C_ec * C_sei_ec * exp()) - C_sei_exp = C_sei_ec * pybamm.exp(-0.5 * prefactor * eta_SEI) + C_sei_exp = C_sei_ec * pybamm.exp(-alpha_SEI * prefactor * eta_SEI) j_sei = -C_sei_exp / (1 + L_sei * C_ec * C_sei_exp) c_ec = 1 / (1 + L_sei * C_ec * C_sei_exp) diff --git a/pybamm/parameters/lithium_ion_parameters.py b/pybamm/parameters/lithium_ion_parameters.py index 7ffe53186c..e8c67f2572 100644 --- a/pybamm/parameters/lithium_ion_parameters.py +++ b/pybamm/parameters/lithium_ion_parameters.py @@ -704,6 +704,9 @@ def _set_dimensional_parameters(self): self.E_sei_dimensional = pybamm.Parameter( f"{pref}SEI growth activation energy [J.mol-1]" ) + self.alpha_SEI = pybamm.Parameter( + f"{pref}SEI growth transfer coefficient" + ) # EC reaction self.c_ec_0_dim = pybamm.Parameter( From e29b43d2567496220dedf11fbd4334dee714508f Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Thu, 3 Nov 2022 11:24:51 +0000 Subject: [PATCH 2/9] style: pre-commit fixes --- pybamm/parameters/lithium_ion_parameters.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/pybamm/parameters/lithium_ion_parameters.py b/pybamm/parameters/lithium_ion_parameters.py index e8c67f2572..811a66a1da 100644 --- a/pybamm/parameters/lithium_ion_parameters.py +++ b/pybamm/parameters/lithium_ion_parameters.py @@ -704,9 +704,7 @@ def _set_dimensional_parameters(self): self.E_sei_dimensional = pybamm.Parameter( f"{pref}SEI growth activation energy [J.mol-1]" ) - self.alpha_SEI = pybamm.Parameter( - f"{pref}SEI growth transfer coefficient" - ) + self.alpha_SEI = pybamm.Parameter(f"{pref}SEI growth transfer coefficient") # EC reaction self.c_ec_0_dim = pybamm.Parameter( From 880b01a49da7c9d4603c5df23a589c72346c9cff Mon Sep 17 00:00:00 2001 From: Ferran Brosa Planella Date: Thu, 3 Nov 2022 14:04:54 +0000 Subject: [PATCH 3/9] #2338 fix options --- benchmarks/different_model_options.py | 12 ++++--- examples/scripts/calendar_ageing.py | 12 ++++--- examples/scripts/cycling_ageing.py | 2 +- .../full_battery_models/base_battery_model.py | 14 +++++--- .../lithium_ion/Yang2017.py | 2 +- .../submodels/interface/sei/base_sei.py | 5 +-- .../submodels/interface/sei/sei_growth.py | 8 ++--- .../base_lithium_ion_half_cell_tests.py | 12 +++++-- .../base_lithium_ion_tests.py | 20 +++++++++-- tests/unit/test_citations.py | 8 +++-- .../test_simulation_with_experiment.py | 4 +-- .../test_base_battery_model.py | 2 +- .../base_lithium_ion_half_cell_tests.py | 12 +++++-- .../base_lithium_ion_tests.py | 36 +++++++++++++++---- .../test_lithium_ion/test_mpm.py | 7 ++-- .../test_lithium_ion/test_spm.py | 2 +- .../test_plot_summary_variables.py | 2 +- 17 files changed, 116 insertions(+), 44 deletions(-) diff --git a/benchmarks/different_model_options.py b/benchmarks/different_model_options.py index 06af900037..051c86de1c 100644 --- a/benchmarks/different_model_options.py +++ b/benchmarks/different_model_options.py @@ -123,11 +123,13 @@ class TimeBuildModelSEI: [ "none", "constant", - "reaction limited", + "reaction limited (symmetric)", + "reaction limited (asymmetric)", "solvent-diffusion limited", "electron-migration limited", "interstitial-diffusion limited", - "ec reaction limited", + "ec reaction limited (symmetric)", + "ec reaction limited (asymmetric)", ], ) @@ -142,11 +144,13 @@ class TimeSolveSEI: [ "none", "constant", - "reaction limited", + "reaction limited (symmetric)", + "reaction limited (asymmetric)", "solvent-diffusion limited", "electron-migration limited", "interstitial-diffusion limited", - "ec reaction limited", + "ec reaction limited (symmetric)", + "ec reaction limited (asymmetric)", ], ) diff --git a/examples/scripts/calendar_ageing.py b/examples/scripts/calendar_ageing.py index 5201f0a7ce..179b16f68b 100644 --- a/examples/scripts/calendar_ageing.py +++ b/examples/scripts/calendar_ageing.py @@ -4,15 +4,17 @@ pb.set_logging_level("INFO") models = [ - pb.lithium_ion.SPM({"SEI": "reaction limited"}), - pb.lithium_ion.SPMe({"SEI": "reaction limited"}), + pb.lithium_ion.SPM({"SEI": "reaction limited (symmetric)"}), + pb.lithium_ion.SPMe({"SEI": "reaction limited (symmetric)"}), pb.lithium_ion.SPM( - {"SEI": "reaction limited", "surface form": "algebraic"}, name="Algebraic SPM" + {"SEI": "reaction limited (symmetric)", "surface form": "algebraic"}, + name="Algebraic SPM", ), pb.lithium_ion.SPMe( - {"SEI": "reaction limited", "surface form": "algebraic"}, name="Algebraic SPMe" + {"SEI": "reaction limited (symmetric)", "surface form": "algebraic"}, + name="Algebraic SPMe", ), - pb.lithium_ion.DFN({"SEI": "reaction limited"}), + pb.lithium_ion.DFN({"SEI": "reaction limited (symmetric)"}), ] sims = [] diff --git a/examples/scripts/cycling_ageing.py b/examples/scripts/cycling_ageing.py index c649e8e7dd..fce5fa6cd6 100644 --- a/examples/scripts/cycling_ageing.py +++ b/examples/scripts/cycling_ageing.py @@ -3,7 +3,7 @@ pb.set_logging_level("NOTICE") model = pb.lithium_ion.DFN( { - "SEI": "ec reaction limited", + "SEI": "ec reaction limited (symmetric)", "SEI film resistance": "distributed", "SEI porosity change": "true", "lithium plating": "irreversible", diff --git a/pybamm/models/full_battery_models/base_battery_model.py b/pybamm/models/full_battery_models/base_battery_model.py index a838f7f4e0..592a332fdd 100644 --- a/pybamm/models/full_battery_models/base_battery_model.py +++ b/pybamm/models/full_battery_models/base_battery_model.py @@ -121,9 +121,11 @@ class BatteryModelOptions(pybamm.FuzzyDict): - "none": :class:`pybamm.sei.NoSEI` (no SEI growth) - "constant": :class:`pybamm.sei.Constant` (constant SEI thickness) - - "reaction limited", "solvent-diffusion limited",\ - "electron-migration limited", "interstitial-diffusion limited", \ - or "ec reaction limited": :class:`pybamm.sei.SEIGrowth` + - "reaction limited (symmetric)", "reaction limited (asymmetric)", \ + "solvent-diffusion limited", "electron-migration limited", \ + "interstitial-diffusion limited", \ + "ec reaction limited (symmetric)" \ + or "ec reaction limited (asymmetric)": :class:`pybamm.sei.SEIGrowth` * "SEI film resistance" : str Set the submodel for additional term in the overpotential due to SEI. The default value is "none" if the "SEI" option is "none", and @@ -254,11 +256,13 @@ def __init__(self, extra_options): "SEI": [ "none", "constant", - "reaction limited", + "reaction limited (symmetric)", + "reaction limited (asymmetric)", "solvent-diffusion limited", "electron-migration limited", "interstitial-diffusion limited", - "ec reaction limited", + "ec reaction limited (symmetric)", + "ec reaction limited (asymmetric)", ], "SEI film resistance": ["none", "distributed", "average"], "SEI on cracks": ["false", "true"], diff --git a/pybamm/models/full_battery_models/lithium_ion/Yang2017.py b/pybamm/models/full_battery_models/lithium_ion/Yang2017.py index f55df43972..36a2c02d59 100644 --- a/pybamm/models/full_battery_models/lithium_ion/Yang2017.py +++ b/pybamm/models/full_battery_models/lithium_ion/Yang2017.py @@ -5,7 +5,7 @@ class Yang2017(DFN): def __init__(self, options=None, name="Yang2017", build=True): options = { - "SEI": "ec reaction limited", + "SEI": "ec reaction limited (symmetric)", "SEI film resistance": "distributed", "SEI porosity change": "true", "lithium plating": "irreversible", diff --git a/pybamm/models/submodels/interface/sei/base_sei.py b/pybamm/models/submodels/interface/sei/base_sei.py index f3f796a86f..2f3744193d 100644 --- a/pybamm/models/submodels/interface/sei/base_sei.py +++ b/pybamm/models/submodels/interface/sei/base_sei.py @@ -198,8 +198,9 @@ def _get_standard_concentration_variables(self, variables): ) v_bar = phase_param.v_bar z_sei = phase_param.z_sei - # Set scales for the "EC Reaction Limited" model - if self.options["SEI"] == "ec reaction limited": + # Set scales for the "EC Reaction Limited" models (both symmetric and + # asymmetric) + if self.options["SEI"].startswith("ec reaction limited"): L_inner_0 = 0 L_outer_0 = 1 L_inner_crack_0 = 0 diff --git a/pybamm/models/submodels/interface/sei/sei_growth.py b/pybamm/models/submodels/interface/sei/sei_growth.py index e82ffe5d94..9e8b812374 100644 --- a/pybamm/models/submodels/interface/sei/sei_growth.py +++ b/pybamm/models/submodels/interface/sei/sei_growth.py @@ -56,7 +56,7 @@ def get_fundamental_variables(self): L_inner, L_outer = Ls - if self.options["SEI"] == "ec reaction limited": + if self.options["SEI"].startswith("ec reaction limited"): L_inner = 0 * L_inner # Set L_inner to zero, copying domains variables = self._get_standard_thickness_variables(L_inner, L_outer) @@ -157,7 +157,7 @@ def get_coupled_variables(self, variables): } ) - if self.options["SEI"] == "ec reaction limited": + if self.options["SEI"].startswith("ec reaction limited"): inner_sei_proportion = 0 else: inner_sei_proportion = phase_param.inner_sei_proportion @@ -231,7 +231,7 @@ def set_rhs(self, variables): Gamma_SEI = self.phase_param.Gamma_SEI - if self.options["SEI"] == "ec reaction limited": + if self.options["SEI"].startswith("ec reaction limited"): self.rhs = {L_outer: -Gamma_SEI * a * j_outer + spreading_outer} else: v_bar = self.phase_param.v_bar @@ -254,7 +254,7 @@ def set_initial_conditions(self, variables): else: L_inner_0 = self.phase_param.L_inner_0 L_outer_0 = self.phase_param.L_outer_0 - if self.options["SEI"] == "ec reaction limited": + if self.options["SEI"].startswith("ec reaction limited"): self.initial_conditions = {L_outer: L_inner_0 + L_outer_0} else: self.initial_conditions = {L_inner: L_inner_0, L_outer: L_outer_0} diff --git a/tests/integration/test_models/test_full_battery_models/test_lithium_ion/base_lithium_ion_half_cell_tests.py b/tests/integration/test_models/test_full_battery_models/test_lithium_ion/base_lithium_ion_half_cell_tests.py index c8055ebcc9..2c30f5952e 100644 --- a/tests/integration/test_models/test_full_battery_models/test_lithium_ion/base_lithium_ion_half_cell_tests.py +++ b/tests/integration/test_models/test_full_battery_models/test_lithium_ion/base_lithium_ion_half_cell_tests.py @@ -50,7 +50,11 @@ def test_sei_constant(self): self.run_basic_processing_test(options) def test_sei_reaction_limited(self): - options = {"SEI": "reaction limited"} + options = {"SEI": "reaction limited (symmetric)"} + self.run_basic_processing_test(options) + + def test_sei_asymmetric_reaction_limited(self): + options = {"SEI": "reaction limited (asymmetric)"} self.run_basic_processing_test(options) def test_sei_solvent_diffusion_limited(self): @@ -66,7 +70,11 @@ def test_sei_interstitial_diffusion_limited(self): self.run_basic_processing_test(options) def test_sei_ec_reaction_limited(self): - options = {"SEI": "ec reaction limited"} + options = {"SEI": "ec reaction limited (symmetric)"} + self.run_basic_processing_test(options) + + def test_sei_asymmetric_ec_reaction_limited(self): + options = {"SEI": "ec reaction limited (asymmetric)"} self.run_basic_processing_test(options) def test_constant_utilisation(self): diff --git a/tests/integration/test_models/test_full_battery_models/test_lithium_ion/base_lithium_ion_tests.py b/tests/integration/test_models/test_full_battery_models/test_lithium_ion/base_lithium_ion_tests.py index 6d46766220..b591dd40aa 100644 --- a/tests/integration/test_models/test_full_battery_models/test_lithium_ion/base_lithium_ion_tests.py +++ b/tests/integration/test_models/test_full_battery_models/test_lithium_ion/base_lithium_ion_tests.py @@ -166,7 +166,11 @@ def test_irreversible_plating_with_porosity(self): self.run_basic_processing_test(options, parameter_values=param) def test_sei_reaction_limited(self): - options = {"SEI": "reaction limited"} + options = {"SEI": "reaction limited (symmetric)"} + self.run_basic_processing_test(options) + + def test_sei_asymmetric_reaction_limited(self): + options = {"SEI": "reaction limited (asymmetric)"} self.run_basic_processing_test(options) def test_sei_solvent_diffusion_limited(self): @@ -182,7 +186,17 @@ def test_sei_interstitial_diffusion_limited(self): self.run_basic_processing_test(options) def test_sei_ec_reaction_limited(self): - options = {"SEI": "ec reaction limited", "SEI porosity change": "true"} + options = { + "SEI": "ec reaction limited (symmetric)", + "SEI porosity change": "true", + } + self.run_basic_processing_test(options) + + def test_sei_asymmetric_ec_reaction_limited(self): + options = { + "SEI": "ec reaction limited (asymmetric)", + "SEI porosity change": "true", + } self.run_basic_processing_test(options) def test_loss_active_material_stress_negative(self): @@ -246,7 +260,7 @@ def test_composite_graphite_silicon_sei(self): options = { "particle phases": ("2", "1"), "open circuit potential": (("single", "current sigmoid"), "single"), - "SEI": "ec reaction limited", + "SEI": "ec reaction limited (symmetric)", } parameter_values = pybamm.ParameterValues("Chen2020_composite") name = "Negative electrode active material volume fraction" diff --git a/tests/unit/test_citations.py b/tests/unit/test_citations.py index ebb2a01538..3409ea4a81 100644 --- a/tests/unit/test_citations.py +++ b/tests/unit/test_citations.py @@ -222,11 +222,15 @@ def test_brosaplanella_2022(self): pybamm.lithium_ion.SPMe(build=False, options={"SEI": "constant"}) self.assertNotIn("BrosaPlanella2022", citations._papers_to_cite) - pybamm.lithium_ion.SPM(build=False, options={"SEI": "ec reaction limited"}) + pybamm.lithium_ion.SPM( + build=False, options={"SEI": "ec reaction limited (symmetric)"} + ) self.assertIn("BrosaPlanella2022", citations._papers_to_cite) citations._reset() - pybamm.lithium_ion.SPMe(build=False, options={"SEI": "ec reaction limited"}) + pybamm.lithium_ion.SPMe( + build=False, options={"SEI": "ec reaction limited (symmetric)"} + ) self.assertIn("BrosaPlanella2022", citations._papers_to_cite) citations._reset() diff --git a/tests/unit/test_experiments/test_simulation_with_experiment.py b/tests/unit/test_experiments/test_simulation_with_experiment.py index 856ec56f7a..ca322ea37e 100644 --- a/tests/unit/test_experiments/test_simulation_with_experiment.py +++ b/tests/unit/test_experiments/test_simulation_with_experiment.py @@ -259,7 +259,7 @@ def test_run_experiment_termination_capacity(self): * 10, termination="99% capacity", ) - model = pybamm.lithium_ion.SPM({"SEI": "ec reaction limited"}) + model = pybamm.lithium_ion.SPM({"SEI": "ec reaction limited (symmetric)"}) param = pybamm.ParameterValues("Chen2020") param["SEI kinetic rate constant [m.s-1]"] = 1e-14 sim = pybamm.Simulation(model, experiment=experiment, parameter_values=param) @@ -281,7 +281,7 @@ def test_run_experiment_termination_capacity(self): * 10, termination="5.04Ah capacity", ) - model = pybamm.lithium_ion.SPM({"SEI": "ec reaction limited"}) + model = pybamm.lithium_ion.SPM({"SEI": "ec reaction limited (symmetric)"}) param = pybamm.ParameterValues("Chen2020") param["SEI kinetic rate constant [m.s-1]"] = 1e-14 sim = pybamm.Simulation(model, experiment=experiment, parameter_values=param) diff --git a/tests/unit/test_models/test_full_battery_models/test_base_battery_model.py b/tests/unit/test_models/test_full_battery_models/test_base_battery_model.py index 2678bca5b0..e13dfd2948 100644 --- a/tests/unit/test_models/test_full_battery_models/test_base_battery_model.py +++ b/tests/unit/test_models/test_full_battery_models/test_base_battery_model.py @@ -34,7 +34,7 @@ 'particle phases': '1' (possible: ['1', '2']) 'particle shape': 'spherical' (possible: ['spherical', 'no particles']) 'particle size': 'single' (possible: ['single', 'distribution']) -'SEI': 'none' (possible: ['none', 'constant', 'reaction limited', 'solvent-diffusion limited', 'electron-migration limited', 'interstitial-diffusion limited', 'ec reaction limited']) +'SEI': 'none' (possible: ['none', 'constant', 'reaction limited (symmetric)', 'reaction limited (asymmetric)', 'solvent-diffusion limited', 'electron-migration limited', 'interstitial-diffusion limited', 'ec reaction limited (symmetric)', 'ec reaction limited (asymmetric)']) 'SEI film resistance': 'none' (possible: ['none', 'distributed', 'average']) 'SEI on cracks': 'false' (possible: ['false', 'true']) 'SEI porosity change': 'false' (possible: ['false', 'true']) diff --git a/tests/unit/test_models/test_full_battery_models/test_lithium_ion/base_lithium_ion_half_cell_tests.py b/tests/unit/test_models/test_full_battery_models/test_lithium_ion/base_lithium_ion_half_cell_tests.py index 1635c2ea41..cdb4cc9ecf 100644 --- a/tests/unit/test_models/test_full_battery_models/test_lithium_ion/base_lithium_ion_half_cell_tests.py +++ b/tests/unit/test_models/test_full_battery_models/test_lithium_ion/base_lithium_ion_half_cell_tests.py @@ -52,7 +52,11 @@ def test_well_posed_constant_sei(self): self.check_well_posedness(options) def test_well_posed_reaction_limited_sei(self): - options = {"SEI": "reaction limited"} + options = {"SEI": "reaction limited (symmetric)"} + self.check_well_posedness(options) + + def test_well_posed_asymmetric_reaction_limited_sei(self): + options = {"SEI": "reaction limited (asymmetric)"} self.check_well_posedness(options) def test_well_posed_solvent_diffusion_limited_sei(self): @@ -68,7 +72,11 @@ def test_well_posed_interstitial_diffusion_limited_sei(self): self.check_well_posedness(options) def test_well_posed_ec_reaction_limited_sei(self): - options = {"SEI": "ec reaction limited"} + options = {"SEI": "ec reaction limited (symmetric)"} + self.check_well_posedness(options) + + def test_well_posed_asymmetric_ec_reaction_limited_sei(self): + options = {"SEI": "ec reaction limited (asymmetric)"} self.check_well_posedness(options) def test_well_posed_lumped_thermal(self): diff --git a/tests/unit/test_models/test_full_battery_models/test_lithium_ion/base_lithium_ion_tests.py b/tests/unit/test_models/test_full_battery_models/test_lithium_ion/base_lithium_ion_tests.py index c3188c2891..a0cfac45d3 100644 --- a/tests/unit/test_models/test_full_battery_models/test_lithium_ion/base_lithium_ion_tests.py +++ b/tests/unit/test_models/test_full_battery_models/test_lithium_ion/base_lithium_ion_tests.py @@ -144,11 +144,25 @@ def test_well_posed_sei_constant(self): self.check_well_posedness(options) def test_well_posed_sei_reaction_limited(self): - options = {"SEI": "reaction limited"} + options = {"SEI": "reaction limited (symmetric)"} + self.check_well_posedness(options) + + def test_well_posed_asymmetric_sei_reaction_limited(self): + options = {"SEI": "reaction limited (asymmetric)"} self.check_well_posedness(options) def test_well_posed_sei_reaction_limited_average_film_resistance(self): - options = {"SEI": "reaction limited", "SEI film resistance": "average"} + options = { + "SEI": "reaction limited (symmetric)", + "SEI film resistance": "average", + } + self.check_well_posedness(options) + + def test_well_posed_asymmetric_sei_reaction_limited_average_film_resistance(self): + options = { + "SEI": "reaction limited (asymmetric)", + "SEI film resistance": "average", + } self.check_well_posedness(options) def test_well_posed_sei_solvent_diffusion_limited(self): @@ -164,7 +178,17 @@ def test_well_posed_sei_interstitial_diffusion_limited(self): self.check_well_posedness(options) def test_well_posed_sei_ec_reaction_limited(self): - options = {"SEI": "ec reaction limited", "SEI porosity change": "true"} + options = { + "SEI": "ec reaction limited (symmetric)", + "SEI porosity change": "true", + } + self.check_well_posedness(options) + + def test_well_posed_sei_asymmetric_ec_reaction_limited(self): + options = { + "SEI": "ec reaction limited (asymmetric)", + "SEI porosity change": "true", + } self.check_well_posedness(options) def test_well_posed_mechanics_negative_cracking(self): @@ -199,7 +223,7 @@ def test_well_posed_mechanics_stress_induced_diffusion_mixed(self): def test_well_posed_sei_reaction_limited_on_cracks(self): options = { - "SEI": "reaction limited", + "SEI": "reaction limited (symmetric)", "SEI on cracks": "true", "particle mechanics": "swelling and cracking", } @@ -231,7 +255,7 @@ def test_well_posed_sei_interstitial_diffusion_limited_on_cracks(self): def test_well_posed_sei_ec_reaction_limited_on_cracks(self): options = { - "SEI": "ec reaction limited", + "SEI": "ec reaction limited (symmetric)", "SEI porosity change": "true", "SEI on cracks": "true", "particle mechanics": "swelling and cracking", @@ -325,7 +349,7 @@ def test_well_posed_particle_phases(self): self.check_well_posedness(options) def test_well_posed_particle_phases_sei(self): - options = {"particle phases": "2", "SEI": "ec reaction limited"} + options = {"particle phases": "2", "SEI": "ec reaction limited (symmetric)"} self.check_well_posedness(options) def test_well_posed_current_sigmoid_ocp(self): diff --git a/tests/unit/test_models/test_full_battery_models/test_lithium_ion/test_mpm.py b/tests/unit/test_models/test_full_battery_models/test_lithium_ion/test_mpm.py index 0798d095c2..0e80dc1413 100644 --- a/tests/unit/test_models/test_full_battery_models/test_lithium_ion/test_mpm.py +++ b/tests/unit/test_models/test_full_battery_models/test_lithium_ion/test_mpm.py @@ -121,7 +121,7 @@ def external_circuit_function(variables): class TestMPMWithSEI(unittest.TestCase): def test_reaction_limited_not_implemented(self): - options = {"SEI": "reaction limited"} + options = {"SEI": "reaction limited (symmetric)"} with self.assertRaises(NotImplementedError): pybamm.lithium_ion.MPM(options) @@ -141,7 +141,10 @@ def test_interstitial_diffusion_limited_not_implemented(self): pybamm.lithium_ion.MPM(options) def test_ec_reaction_limited_not_implemented(self): - options = {"SEI": "ec reaction limited", "SEI porosity change": "true"} + options = { + "SEI": "ec reaction limited (symmetric)", + "SEI porosity change": "true", + } with self.assertRaises(NotImplementedError): pybamm.lithium_ion.MPM(options) diff --git a/tests/unit/test_models/test_full_battery_models/test_lithium_ion/test_spm.py b/tests/unit/test_models/test_full_battery_models/test_lithium_ion/test_spm.py index ee5dffe25c..4e26e30fa3 100644 --- a/tests/unit/test_models/test_full_battery_models/test_lithium_ion/test_spm.py +++ b/tests/unit/test_models/test_full_battery_models/test_lithium_ion/test_spm.py @@ -28,7 +28,7 @@ def test_x_average_options(self): options = { "lithium plating": "irreversible", "lithium plating porosity change": "true", - "SEI": "ec reaction limited", + "SEI": "ec reaction limited (symmetric)", "SEI porosity change": "true", "x-average side reactions": "true", } diff --git a/tests/unit/test_plotting/test_plot_summary_variables.py b/tests/unit/test_plotting/test_plot_summary_variables.py index 331d5615df..1960895c08 100644 --- a/tests/unit/test_plotting/test_plot_summary_variables.py +++ b/tests/unit/test_plotting/test_plot_summary_variables.py @@ -5,7 +5,7 @@ class TestPlotSummaryVariables(unittest.TestCase): def test_plot(self): - model = pybamm.lithium_ion.SPM({"SEI": "ec reaction limited"}) + model = pybamm.lithium_ion.SPM({"SEI": "ec reaction limited (symmetric)"}) parameter_values = pybamm.ParameterValues("Mohtat2020") experiment = pybamm.Experiment( [ From 4941a33a6dc95de478e9705a365d7664d8619a2f Mon Sep 17 00:00:00 2001 From: Ferran Brosa Planella Date: Thu, 3 Nov 2022 16:34:07 +0000 Subject: [PATCH 4/9] #2338 fix integration tests --- .../base_lithium_ion_half_cell_tests.py | 18 ++++++++++++++++-- .../test_lithium_ion/base_lithium_ion_tests.py | 18 ++++++++++++++++-- 2 files changed, 32 insertions(+), 4 deletions(-) diff --git a/tests/integration/test_models/test_full_battery_models/test_lithium_ion/base_lithium_ion_half_cell_tests.py b/tests/integration/test_models/test_full_battery_models/test_lithium_ion/base_lithium_ion_half_cell_tests.py index 2c30f5952e..b74bdb0a61 100644 --- a/tests/integration/test_models/test_full_battery_models/test_lithium_ion/base_lithium_ion_half_cell_tests.py +++ b/tests/integration/test_models/test_full_battery_models/test_lithium_ion/base_lithium_ion_half_cell_tests.py @@ -55,7 +55,14 @@ def test_sei_reaction_limited(self): def test_sei_asymmetric_reaction_limited(self): options = {"SEI": "reaction limited (asymmetric)"} - self.run_basic_processing_test(options) + parameter_values = pybamm.ParameterValues("Xu2019") + parameter_values.update( + { + "SEI growth transfer coefficient": 0.2 + }, + check_already_exists=False, + ) + self.run_basic_processing_test(options, parameter_values=parameter_values) def test_sei_solvent_diffusion_limited(self): options = {"SEI": "solvent-diffusion limited"} @@ -75,7 +82,14 @@ def test_sei_ec_reaction_limited(self): def test_sei_asymmetric_ec_reaction_limited(self): options = {"SEI": "ec reaction limited (asymmetric)"} - self.run_basic_processing_test(options) + parameter_values = pybamm.ParameterValues("Xu2019") + parameter_values.update( + { + "SEI growth transfer coefficient": 0.2 + }, + check_already_exists=False, + ) + self.run_basic_processing_test(options, parameter_values=parameter_values) def test_constant_utilisation(self): options = {"interface utilisation": "constant"} diff --git a/tests/integration/test_models/test_full_battery_models/test_lithium_ion/base_lithium_ion_tests.py b/tests/integration/test_models/test_full_battery_models/test_lithium_ion/base_lithium_ion_tests.py index b591dd40aa..7719bc399b 100644 --- a/tests/integration/test_models/test_full_battery_models/test_lithium_ion/base_lithium_ion_tests.py +++ b/tests/integration/test_models/test_full_battery_models/test_lithium_ion/base_lithium_ion_tests.py @@ -171,7 +171,14 @@ def test_sei_reaction_limited(self): def test_sei_asymmetric_reaction_limited(self): options = {"SEI": "reaction limited (asymmetric)"} - self.run_basic_processing_test(options) + parameter_values = pybamm.ParameterValues("Xu2019") + parameter_values.update( + { + "SEI growth transfer coefficient": 0.2 + }, + check_already_exists=False, + ) + self.run_basic_processing_test(options, parameter_values=parameter_values) def test_sei_solvent_diffusion_limited(self): options = {"SEI": "solvent-diffusion limited"} @@ -197,7 +204,14 @@ def test_sei_asymmetric_ec_reaction_limited(self): "SEI": "ec reaction limited (asymmetric)", "SEI porosity change": "true", } - self.run_basic_processing_test(options) + parameter_values = pybamm.ParameterValues("Xu2019") + parameter_values.update( + { + "SEI growth transfer coefficient": 0.2 + }, + check_already_exists=False, + ) + self.run_basic_processing_test(options, parameter_values=parameter_values) def test_loss_active_material_stress_negative(self): options = {"loss of active material": ("none", "stress-driven")} From 1413d5ce3c3598bdb161b71944fd509ec5823b3d Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Thu, 3 Nov 2022 16:35:39 +0000 Subject: [PATCH 5/9] style: pre-commit fixes --- .../test_lithium_ion/base_lithium_ion_half_cell_tests.py | 8 ++------ .../test_lithium_ion/base_lithium_ion_tests.py | 8 ++------ 2 files changed, 4 insertions(+), 12 deletions(-) diff --git a/tests/integration/test_models/test_full_battery_models/test_lithium_ion/base_lithium_ion_half_cell_tests.py b/tests/integration/test_models/test_full_battery_models/test_lithium_ion/base_lithium_ion_half_cell_tests.py index b74bdb0a61..faf1d6d687 100644 --- a/tests/integration/test_models/test_full_battery_models/test_lithium_ion/base_lithium_ion_half_cell_tests.py +++ b/tests/integration/test_models/test_full_battery_models/test_lithium_ion/base_lithium_ion_half_cell_tests.py @@ -57,9 +57,7 @@ def test_sei_asymmetric_reaction_limited(self): options = {"SEI": "reaction limited (asymmetric)"} parameter_values = pybamm.ParameterValues("Xu2019") parameter_values.update( - { - "SEI growth transfer coefficient": 0.2 - }, + {"SEI growth transfer coefficient": 0.2}, check_already_exists=False, ) self.run_basic_processing_test(options, parameter_values=parameter_values) @@ -84,9 +82,7 @@ def test_sei_asymmetric_ec_reaction_limited(self): options = {"SEI": "ec reaction limited (asymmetric)"} parameter_values = pybamm.ParameterValues("Xu2019") parameter_values.update( - { - "SEI growth transfer coefficient": 0.2 - }, + {"SEI growth transfer coefficient": 0.2}, check_already_exists=False, ) self.run_basic_processing_test(options, parameter_values=parameter_values) diff --git a/tests/integration/test_models/test_full_battery_models/test_lithium_ion/base_lithium_ion_tests.py b/tests/integration/test_models/test_full_battery_models/test_lithium_ion/base_lithium_ion_tests.py index 7719bc399b..2e4071130c 100644 --- a/tests/integration/test_models/test_full_battery_models/test_lithium_ion/base_lithium_ion_tests.py +++ b/tests/integration/test_models/test_full_battery_models/test_lithium_ion/base_lithium_ion_tests.py @@ -173,9 +173,7 @@ def test_sei_asymmetric_reaction_limited(self): options = {"SEI": "reaction limited (asymmetric)"} parameter_values = pybamm.ParameterValues("Xu2019") parameter_values.update( - { - "SEI growth transfer coefficient": 0.2 - }, + {"SEI growth transfer coefficient": 0.2}, check_already_exists=False, ) self.run_basic_processing_test(options, parameter_values=parameter_values) @@ -206,9 +204,7 @@ def test_sei_asymmetric_ec_reaction_limited(self): } parameter_values = pybamm.ParameterValues("Xu2019") parameter_values.update( - { - "SEI growth transfer coefficient": 0.2 - }, + {"SEI growth transfer coefficient": 0.2}, check_already_exists=False, ) self.run_basic_processing_test(options, parameter_values=parameter_values) From 4bea6fd73c732bee90df45eff830f7070667f53a Mon Sep 17 00:00:00 2001 From: Ferran Brosa Planella Date: Thu, 3 Nov 2022 18:01:41 +0000 Subject: [PATCH 6/9] #2338 fix failing tests --- .../test_lithium_ion/base_lithium_ion_tests.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/integration/test_models/test_full_battery_models/test_lithium_ion/base_lithium_ion_tests.py b/tests/integration/test_models/test_full_battery_models/test_lithium_ion/base_lithium_ion_tests.py index 7719bc399b..c290afc482 100644 --- a/tests/integration/test_models/test_full_battery_models/test_lithium_ion/base_lithium_ion_tests.py +++ b/tests/integration/test_models/test_full_battery_models/test_lithium_ion/base_lithium_ion_tests.py @@ -171,7 +171,7 @@ def test_sei_reaction_limited(self): def test_sei_asymmetric_reaction_limited(self): options = {"SEI": "reaction limited (asymmetric)"} - parameter_values = pybamm.ParameterValues("Xu2019") + parameter_values = pybamm.ParameterValues("Marquis2019") parameter_values.update( { "SEI growth transfer coefficient": 0.2 @@ -204,7 +204,7 @@ def test_sei_asymmetric_ec_reaction_limited(self): "SEI": "ec reaction limited (asymmetric)", "SEI porosity change": "true", } - parameter_values = pybamm.ParameterValues("Xu2019") + parameter_values = pybamm.ParameterValues("Marquis2019") parameter_values.update( { "SEI growth transfer coefficient": 0.2 From 404031c4145527c45dcd4f6e7e973bcde34bf3f4 Mon Sep 17 00:00:00 2001 From: Ferran Brosa Planella Date: Thu, 3 Nov 2022 19:12:34 +0000 Subject: [PATCH 7/9] #2338 revert breaking changes --- benchmarks/different_model_options.py | 8 ++++---- examples/scripts/calendar_ageing.py | 10 +++++----- examples/scripts/cycling_ageing.py | 2 +- .../models/full_battery_models/base_battery_model.py | 9 ++++----- .../full_battery_models/lithium_ion/Yang2017.py | 2 +- .../base_lithium_ion_half_cell_tests.py | 4 ++-- .../test_lithium_ion/base_lithium_ion_tests.py | 6 +++--- tests/unit/test_citations.py | 4 ++-- .../test_simulation_with_experiment.py | 4 ++-- .../test_base_battery_model.py | 2 +- .../base_lithium_ion_half_cell_tests.py | 4 ++-- .../test_lithium_ion/base_lithium_ion_tests.py | 12 ++++++------ .../test_lithium_ion/test_mpm.py | 4 ++-- .../test_lithium_ion/test_spm.py | 2 +- .../test_plotting/test_plot_summary_variables.py | 2 +- 15 files changed, 37 insertions(+), 38 deletions(-) diff --git a/benchmarks/different_model_options.py b/benchmarks/different_model_options.py index 051c86de1c..b328398177 100644 --- a/benchmarks/different_model_options.py +++ b/benchmarks/different_model_options.py @@ -123,12 +123,12 @@ class TimeBuildModelSEI: [ "none", "constant", - "reaction limited (symmetric)", + "reaction limited", "reaction limited (asymmetric)", "solvent-diffusion limited", "electron-migration limited", "interstitial-diffusion limited", - "ec reaction limited (symmetric)", + "ec reaction limited", "ec reaction limited (asymmetric)", ], ) @@ -144,12 +144,12 @@ class TimeSolveSEI: [ "none", "constant", - "reaction limited (symmetric)", + "reaction limited", "reaction limited (asymmetric)", "solvent-diffusion limited", "electron-migration limited", "interstitial-diffusion limited", - "ec reaction limited (symmetric)", + "ec reaction limited", "ec reaction limited (asymmetric)", ], ) diff --git a/examples/scripts/calendar_ageing.py b/examples/scripts/calendar_ageing.py index 179b16f68b..b33b4d35c7 100644 --- a/examples/scripts/calendar_ageing.py +++ b/examples/scripts/calendar_ageing.py @@ -4,17 +4,17 @@ pb.set_logging_level("INFO") models = [ - pb.lithium_ion.SPM({"SEI": "reaction limited (symmetric)"}), - pb.lithium_ion.SPMe({"SEI": "reaction limited (symmetric)"}), + pb.lithium_ion.SPM({"SEI": "reaction limited"}), + pb.lithium_ion.SPMe({"SEI": "reaction limited"}), pb.lithium_ion.SPM( - {"SEI": "reaction limited (symmetric)", "surface form": "algebraic"}, + {"SEI": "reaction limited", "surface form": "algebraic"}, name="Algebraic SPM", ), pb.lithium_ion.SPMe( - {"SEI": "reaction limited (symmetric)", "surface form": "algebraic"}, + {"SEI": "reaction limited", "surface form": "algebraic"}, name="Algebraic SPMe", ), - pb.lithium_ion.DFN({"SEI": "reaction limited (symmetric)"}), + pb.lithium_ion.DFN({"SEI": "reaction limited"}), ] sims = [] diff --git a/examples/scripts/cycling_ageing.py b/examples/scripts/cycling_ageing.py index fce5fa6cd6..c649e8e7dd 100644 --- a/examples/scripts/cycling_ageing.py +++ b/examples/scripts/cycling_ageing.py @@ -3,7 +3,7 @@ pb.set_logging_level("NOTICE") model = pb.lithium_ion.DFN( { - "SEI": "ec reaction limited (symmetric)", + "SEI": "ec reaction limited", "SEI film resistance": "distributed", "SEI porosity change": "true", "lithium plating": "irreversible", diff --git a/pybamm/models/full_battery_models/base_battery_model.py b/pybamm/models/full_battery_models/base_battery_model.py index 592a332fdd..ccb47e7454 100644 --- a/pybamm/models/full_battery_models/base_battery_model.py +++ b/pybamm/models/full_battery_models/base_battery_model.py @@ -121,10 +121,9 @@ class BatteryModelOptions(pybamm.FuzzyDict): - "none": :class:`pybamm.sei.NoSEI` (no SEI growth) - "constant": :class:`pybamm.sei.Constant` (constant SEI thickness) - - "reaction limited (symmetric)", "reaction limited (asymmetric)", \ + - "reaction limited", "reaction limited (asymmetric)", \ "solvent-diffusion limited", "electron-migration limited", \ - "interstitial-diffusion limited", \ - "ec reaction limited (symmetric)" \ + "interstitial-diffusion limited", "ec reaction limited" \ or "ec reaction limited (asymmetric)": :class:`pybamm.sei.SEIGrowth` * "SEI film resistance" : str Set the submodel for additional term in the overpotential due to SEI. @@ -256,12 +255,12 @@ def __init__(self, extra_options): "SEI": [ "none", "constant", - "reaction limited (symmetric)", + "reaction limited", "reaction limited (asymmetric)", "solvent-diffusion limited", "electron-migration limited", "interstitial-diffusion limited", - "ec reaction limited (symmetric)", + "ec reaction limited", "ec reaction limited (asymmetric)", ], "SEI film resistance": ["none", "distributed", "average"], diff --git a/pybamm/models/full_battery_models/lithium_ion/Yang2017.py b/pybamm/models/full_battery_models/lithium_ion/Yang2017.py index 36a2c02d59..f55df43972 100644 --- a/pybamm/models/full_battery_models/lithium_ion/Yang2017.py +++ b/pybamm/models/full_battery_models/lithium_ion/Yang2017.py @@ -5,7 +5,7 @@ class Yang2017(DFN): def __init__(self, options=None, name="Yang2017", build=True): options = { - "SEI": "ec reaction limited (symmetric)", + "SEI": "ec reaction limited", "SEI film resistance": "distributed", "SEI porosity change": "true", "lithium plating": "irreversible", diff --git a/tests/integration/test_models/test_full_battery_models/test_lithium_ion/base_lithium_ion_half_cell_tests.py b/tests/integration/test_models/test_full_battery_models/test_lithium_ion/base_lithium_ion_half_cell_tests.py index faf1d6d687..0c203c9fc7 100644 --- a/tests/integration/test_models/test_full_battery_models/test_lithium_ion/base_lithium_ion_half_cell_tests.py +++ b/tests/integration/test_models/test_full_battery_models/test_lithium_ion/base_lithium_ion_half_cell_tests.py @@ -50,7 +50,7 @@ def test_sei_constant(self): self.run_basic_processing_test(options) def test_sei_reaction_limited(self): - options = {"SEI": "reaction limited (symmetric)"} + options = {"SEI": "reaction limited"} self.run_basic_processing_test(options) def test_sei_asymmetric_reaction_limited(self): @@ -75,7 +75,7 @@ def test_sei_interstitial_diffusion_limited(self): self.run_basic_processing_test(options) def test_sei_ec_reaction_limited(self): - options = {"SEI": "ec reaction limited (symmetric)"} + options = {"SEI": "ec reaction limited"} self.run_basic_processing_test(options) def test_sei_asymmetric_ec_reaction_limited(self): diff --git a/tests/integration/test_models/test_full_battery_models/test_lithium_ion/base_lithium_ion_tests.py b/tests/integration/test_models/test_full_battery_models/test_lithium_ion/base_lithium_ion_tests.py index c1f1ee99a6..33766b4855 100644 --- a/tests/integration/test_models/test_full_battery_models/test_lithium_ion/base_lithium_ion_tests.py +++ b/tests/integration/test_models/test_full_battery_models/test_lithium_ion/base_lithium_ion_tests.py @@ -166,7 +166,7 @@ def test_irreversible_plating_with_porosity(self): self.run_basic_processing_test(options, parameter_values=param) def test_sei_reaction_limited(self): - options = {"SEI": "reaction limited (symmetric)"} + options = {"SEI": "reaction limited"} self.run_basic_processing_test(options) def test_sei_asymmetric_reaction_limited(self): @@ -192,7 +192,7 @@ def test_sei_interstitial_diffusion_limited(self): def test_sei_ec_reaction_limited(self): options = { - "SEI": "ec reaction limited (symmetric)", + "SEI": "ec reaction limited", "SEI porosity change": "true", } self.run_basic_processing_test(options) @@ -270,7 +270,7 @@ def test_composite_graphite_silicon_sei(self): options = { "particle phases": ("2", "1"), "open circuit potential": (("single", "current sigmoid"), "single"), - "SEI": "ec reaction limited (symmetric)", + "SEI": "ec reaction limited", } parameter_values = pybamm.ParameterValues("Chen2020_composite") name = "Negative electrode active material volume fraction" diff --git a/tests/unit/test_citations.py b/tests/unit/test_citations.py index 3409ea4a81..b3142aa495 100644 --- a/tests/unit/test_citations.py +++ b/tests/unit/test_citations.py @@ -223,13 +223,13 @@ def test_brosaplanella_2022(self): self.assertNotIn("BrosaPlanella2022", citations._papers_to_cite) pybamm.lithium_ion.SPM( - build=False, options={"SEI": "ec reaction limited (symmetric)"} + build=False, options={"SEI": "ec reaction limited"} ) self.assertIn("BrosaPlanella2022", citations._papers_to_cite) citations._reset() pybamm.lithium_ion.SPMe( - build=False, options={"SEI": "ec reaction limited (symmetric)"} + build=False, options={"SEI": "ec reaction limited"} ) self.assertIn("BrosaPlanella2022", citations._papers_to_cite) citations._reset() diff --git a/tests/unit/test_experiments/test_simulation_with_experiment.py b/tests/unit/test_experiments/test_simulation_with_experiment.py index ca322ea37e..856ec56f7a 100644 --- a/tests/unit/test_experiments/test_simulation_with_experiment.py +++ b/tests/unit/test_experiments/test_simulation_with_experiment.py @@ -259,7 +259,7 @@ def test_run_experiment_termination_capacity(self): * 10, termination="99% capacity", ) - model = pybamm.lithium_ion.SPM({"SEI": "ec reaction limited (symmetric)"}) + model = pybamm.lithium_ion.SPM({"SEI": "ec reaction limited"}) param = pybamm.ParameterValues("Chen2020") param["SEI kinetic rate constant [m.s-1]"] = 1e-14 sim = pybamm.Simulation(model, experiment=experiment, parameter_values=param) @@ -281,7 +281,7 @@ def test_run_experiment_termination_capacity(self): * 10, termination="5.04Ah capacity", ) - model = pybamm.lithium_ion.SPM({"SEI": "ec reaction limited (symmetric)"}) + model = pybamm.lithium_ion.SPM({"SEI": "ec reaction limited"}) param = pybamm.ParameterValues("Chen2020") param["SEI kinetic rate constant [m.s-1]"] = 1e-14 sim = pybamm.Simulation(model, experiment=experiment, parameter_values=param) diff --git a/tests/unit/test_models/test_full_battery_models/test_base_battery_model.py b/tests/unit/test_models/test_full_battery_models/test_base_battery_model.py index e13dfd2948..b2fa082f7e 100644 --- a/tests/unit/test_models/test_full_battery_models/test_base_battery_model.py +++ b/tests/unit/test_models/test_full_battery_models/test_base_battery_model.py @@ -34,7 +34,7 @@ 'particle phases': '1' (possible: ['1', '2']) 'particle shape': 'spherical' (possible: ['spherical', 'no particles']) 'particle size': 'single' (possible: ['single', 'distribution']) -'SEI': 'none' (possible: ['none', 'constant', 'reaction limited (symmetric)', 'reaction limited (asymmetric)', 'solvent-diffusion limited', 'electron-migration limited', 'interstitial-diffusion limited', 'ec reaction limited (symmetric)', 'ec reaction limited (asymmetric)']) +'SEI': 'none' (possible: ['none', 'constant', 'reaction limited', 'reaction limited (asymmetric)', 'solvent-diffusion limited', 'electron-migration limited', 'interstitial-diffusion limited', 'ec reaction limited', 'ec reaction limited (asymmetric)']) 'SEI film resistance': 'none' (possible: ['none', 'distributed', 'average']) 'SEI on cracks': 'false' (possible: ['false', 'true']) 'SEI porosity change': 'false' (possible: ['false', 'true']) diff --git a/tests/unit/test_models/test_full_battery_models/test_lithium_ion/base_lithium_ion_half_cell_tests.py b/tests/unit/test_models/test_full_battery_models/test_lithium_ion/base_lithium_ion_half_cell_tests.py index cdb4cc9ecf..24ccd55d79 100644 --- a/tests/unit/test_models/test_full_battery_models/test_lithium_ion/base_lithium_ion_half_cell_tests.py +++ b/tests/unit/test_models/test_full_battery_models/test_lithium_ion/base_lithium_ion_half_cell_tests.py @@ -52,7 +52,7 @@ def test_well_posed_constant_sei(self): self.check_well_posedness(options) def test_well_posed_reaction_limited_sei(self): - options = {"SEI": "reaction limited (symmetric)"} + options = {"SEI": "reaction limited"} self.check_well_posedness(options) def test_well_posed_asymmetric_reaction_limited_sei(self): @@ -72,7 +72,7 @@ def test_well_posed_interstitial_diffusion_limited_sei(self): self.check_well_posedness(options) def test_well_posed_ec_reaction_limited_sei(self): - options = {"SEI": "ec reaction limited (symmetric)"} + options = {"SEI": "ec reaction limited"} self.check_well_posedness(options) def test_well_posed_asymmetric_ec_reaction_limited_sei(self): diff --git a/tests/unit/test_models/test_full_battery_models/test_lithium_ion/base_lithium_ion_tests.py b/tests/unit/test_models/test_full_battery_models/test_lithium_ion/base_lithium_ion_tests.py index a0cfac45d3..7dbe128221 100644 --- a/tests/unit/test_models/test_full_battery_models/test_lithium_ion/base_lithium_ion_tests.py +++ b/tests/unit/test_models/test_full_battery_models/test_lithium_ion/base_lithium_ion_tests.py @@ -144,7 +144,7 @@ def test_well_posed_sei_constant(self): self.check_well_posedness(options) def test_well_posed_sei_reaction_limited(self): - options = {"SEI": "reaction limited (symmetric)"} + options = {"SEI": "reaction limited"} self.check_well_posedness(options) def test_well_posed_asymmetric_sei_reaction_limited(self): @@ -153,7 +153,7 @@ def test_well_posed_asymmetric_sei_reaction_limited(self): def test_well_posed_sei_reaction_limited_average_film_resistance(self): options = { - "SEI": "reaction limited (symmetric)", + "SEI": "reaction limited", "SEI film resistance": "average", } self.check_well_posedness(options) @@ -179,7 +179,7 @@ def test_well_posed_sei_interstitial_diffusion_limited(self): def test_well_posed_sei_ec_reaction_limited(self): options = { - "SEI": "ec reaction limited (symmetric)", + "SEI": "ec reaction limited", "SEI porosity change": "true", } self.check_well_posedness(options) @@ -223,7 +223,7 @@ def test_well_posed_mechanics_stress_induced_diffusion_mixed(self): def test_well_posed_sei_reaction_limited_on_cracks(self): options = { - "SEI": "reaction limited (symmetric)", + "SEI": "reaction limited", "SEI on cracks": "true", "particle mechanics": "swelling and cracking", } @@ -255,7 +255,7 @@ def test_well_posed_sei_interstitial_diffusion_limited_on_cracks(self): def test_well_posed_sei_ec_reaction_limited_on_cracks(self): options = { - "SEI": "ec reaction limited (symmetric)", + "SEI": "ec reaction limited", "SEI porosity change": "true", "SEI on cracks": "true", "particle mechanics": "swelling and cracking", @@ -349,7 +349,7 @@ def test_well_posed_particle_phases(self): self.check_well_posedness(options) def test_well_posed_particle_phases_sei(self): - options = {"particle phases": "2", "SEI": "ec reaction limited (symmetric)"} + options = {"particle phases": "2", "SEI": "ec reaction limited"} self.check_well_posedness(options) def test_well_posed_current_sigmoid_ocp(self): diff --git a/tests/unit/test_models/test_full_battery_models/test_lithium_ion/test_mpm.py b/tests/unit/test_models/test_full_battery_models/test_lithium_ion/test_mpm.py index 0e80dc1413..5c824164ea 100644 --- a/tests/unit/test_models/test_full_battery_models/test_lithium_ion/test_mpm.py +++ b/tests/unit/test_models/test_full_battery_models/test_lithium_ion/test_mpm.py @@ -121,7 +121,7 @@ def external_circuit_function(variables): class TestMPMWithSEI(unittest.TestCase): def test_reaction_limited_not_implemented(self): - options = {"SEI": "reaction limited (symmetric)"} + options = {"SEI": "reaction limited"} with self.assertRaises(NotImplementedError): pybamm.lithium_ion.MPM(options) @@ -142,7 +142,7 @@ def test_interstitial_diffusion_limited_not_implemented(self): def test_ec_reaction_limited_not_implemented(self): options = { - "SEI": "ec reaction limited (symmetric)", + "SEI": "ec reaction limited", "SEI porosity change": "true", } with self.assertRaises(NotImplementedError): diff --git a/tests/unit/test_models/test_full_battery_models/test_lithium_ion/test_spm.py b/tests/unit/test_models/test_full_battery_models/test_lithium_ion/test_spm.py index 4e26e30fa3..ee5dffe25c 100644 --- a/tests/unit/test_models/test_full_battery_models/test_lithium_ion/test_spm.py +++ b/tests/unit/test_models/test_full_battery_models/test_lithium_ion/test_spm.py @@ -28,7 +28,7 @@ def test_x_average_options(self): options = { "lithium plating": "irreversible", "lithium plating porosity change": "true", - "SEI": "ec reaction limited (symmetric)", + "SEI": "ec reaction limited", "SEI porosity change": "true", "x-average side reactions": "true", } diff --git a/tests/unit/test_plotting/test_plot_summary_variables.py b/tests/unit/test_plotting/test_plot_summary_variables.py index 1960895c08..331d5615df 100644 --- a/tests/unit/test_plotting/test_plot_summary_variables.py +++ b/tests/unit/test_plotting/test_plot_summary_variables.py @@ -5,7 +5,7 @@ class TestPlotSummaryVariables(unittest.TestCase): def test_plot(self): - model = pybamm.lithium_ion.SPM({"SEI": "ec reaction limited (symmetric)"}) + model = pybamm.lithium_ion.SPM({"SEI": "ec reaction limited"}) parameter_values = pybamm.ParameterValues("Mohtat2020") experiment = pybamm.Experiment( [ From d0fb15d0241b7889d70a89b8b7f1455446c11650 Mon Sep 17 00:00:00 2001 From: Ferran Brosa Planella Date: Thu, 3 Nov 2022 19:14:16 +0000 Subject: [PATCH 8/9] #2338 update CHANGELOG --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index c2ee1d24db..5608867896 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,7 @@ ## Features +- SEI reactions can now be asymmetric ([#2425](https://github.com/pybamm-team/PyBaMM/pull/2425)) - Third-party parameter sets can be added by registering entry points to `pybamm_parameter_set` ([#2396](https://github.com/pybamm-team/PyBaMM/pull/2396)) - Added three-dimensional interpolation ([#2380](https://github.com/pybamm-team/PyBaMM/pull/2380)) From c7b522ea7d9144e4cbf4852fb2e8a0c0c2d188a6 Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Thu, 3 Nov 2022 19:15:21 +0000 Subject: [PATCH 9/9] style: pre-commit fixes --- tests/unit/test_citations.py | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/tests/unit/test_citations.py b/tests/unit/test_citations.py index b3142aa495..ebb2a01538 100644 --- a/tests/unit/test_citations.py +++ b/tests/unit/test_citations.py @@ -222,15 +222,11 @@ def test_brosaplanella_2022(self): pybamm.lithium_ion.SPMe(build=False, options={"SEI": "constant"}) self.assertNotIn("BrosaPlanella2022", citations._papers_to_cite) - pybamm.lithium_ion.SPM( - build=False, options={"SEI": "ec reaction limited"} - ) + pybamm.lithium_ion.SPM(build=False, options={"SEI": "ec reaction limited"}) self.assertIn("BrosaPlanella2022", citations._papers_to_cite) citations._reset() - pybamm.lithium_ion.SPMe( - build=False, options={"SEI": "ec reaction limited"} - ) + pybamm.lithium_ion.SPMe(build=False, options={"SEI": "ec reaction limited"}) self.assertIn("BrosaPlanella2022", citations._papers_to_cite) citations._reset()