From 27e41bef7d53e17b4abbbbca257b9e6ea54d90d2 Mon Sep 17 00:00:00 2001 From: Robert Timms Date: Wed, 7 Dec 2022 23:18:26 +0000 Subject: [PATCH 1/4] allow differential surface form with MPM --- pybamm/models/full_battery_models/lithium_ion/mpm.py | 9 +-------- .../test_lithium_ion/test_mpm.py | 3 --- 2 files changed, 1 insertion(+), 11 deletions(-) diff --git a/pybamm/models/full_battery_models/lithium_ion/mpm.py b/pybamm/models/full_battery_models/lithium_ion/mpm.py index 1320d72932..dc1f61ad80 100644 --- a/pybamm/models/full_battery_models/lithium_ion/mpm.py +++ b/pybamm/models/full_battery_models/lithium_ion/mpm.py @@ -42,22 +42,15 @@ class MPM(SPM): def __init__(self, options=None, name="Many-Particle Model", build=True): # Necessary options if options is None: - options = {"particle size": "distribution", "surface form": "algebraic"} + options = {"particle size": "distribution"} elif "particle size" in options and options["particle size"] != "distribution": raise pybamm.OptionError( "particle size must be 'distribution' for MPM not '{}'".format( options["particle size"] ) ) - elif "surface form" in options and options["surface form"] != "algebraic": - raise pybamm.OptionError( - "surface form must be 'algebraic' for MPM not '{}'".format( - options["surface form"] - ) - ) else: options["particle size"] = "distribution" - options["surface form"] = "algebraic" super().__init__(options, name, build) pybamm.citations.register("Kirk2020") 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 5c824164ea..a80b7053b9 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 @@ -59,9 +59,6 @@ def test_necessary_options(self): options = {"particle size": "single"} with self.assertRaises(pybamm.OptionError): pybamm.lithium_ion.MPM(options) - options = {"surface form": "none"} - with self.assertRaises(pybamm.OptionError): - pybamm.lithium_ion.MPM(options) def test_nonspherical_particle_not_implemented(self): options = {"particle shape": "user"} From 44ac24d060a99b2ea38f5adb6979d5ccaccc646c Mon Sep 17 00:00:00 2001 From: Robert Timms Date: Thu, 8 Dec 2022 10:06:08 +0000 Subject: [PATCH 2/4] MPM w/capacitance tests --- CHANGELOG.md | 3 +++ .../full_battery_models/lithium_ion/mpm.py | 23 +++++++------------ .../test_lithium_ion/test_mpm.py | 6 +++++ .../test_lithium_ion/test_mpm.py | 9 ++++++++ 4 files changed, 26 insertions(+), 15 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b1f757d68a..a361238e93 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,8 @@ # [Unreleased](https://github.com/pybamm-team/PyBaMM/) +## Features +- Allow the option "surface form" to be "differential" in the `MPM` ([#2533](https://github.com/pybamm-team/PyBaMM/pull/2533)) + ## Bug fixes - Fix installation on `Google Colab` (`pybtex` and `Colab` issue) ([#2526](https://github.com/pybamm-team/PyBaMM/pull/2526)) diff --git a/pybamm/models/full_battery_models/lithium_ion/mpm.py b/pybamm/models/full_battery_models/lithium_ion/mpm.py index dc1f61ad80..1e68b718fa 100644 --- a/pybamm/models/full_battery_models/lithium_ion/mpm.py +++ b/pybamm/models/full_battery_models/lithium_ion/mpm.py @@ -40,34 +40,27 @@ class MPM(SPM): """ def __init__(self, options=None, name="Many-Particle Model", build=True): - # Necessary options + # Necessary/default options + default_options = {"particle size": "distribution", "surface form": "algebraic"} if options is None: - options = {"particle size": "distribution"} + options = default_options elif "particle size" in options and options["particle size"] != "distribution": raise pybamm.OptionError( "particle size must be 'distribution' for MPM not '{}'".format( options["particle size"] ) ) + elif "surface form" in options and options["surface form"] == "false": + raise pybamm.OptionError( + "surface form must be 'algebraic' or 'differential' for MPM not 'false'" + ) else: - options["particle size"] = "distribution" + options.update(default_options) super().__init__(options, name, build) pybamm.citations.register("Kirk2020") pybamm.citations.register("Kirk2021") - def set_particle_submodel(self): - for domain in ["negative", "positive"]: - if self.options["particle"] == "Fickian diffusion": - submod = pybamm.particle.FickianDiffusion( - self.param, domain, self.options, x_average=True, phase="primary" - ) - elif self.options["particle"] == "uniform profile": - submod = pybamm.particle.XAveragedPolynomialProfile( - self.param, domain, self.options, phase="primary" - ) - self.submodels[f"{domain} particle"] = submod - @property def default_parameter_values(self): default_params = super().default_parameter_values diff --git a/tests/integration/test_models/test_full_battery_models/test_lithium_ion/test_mpm.py b/tests/integration/test_models/test_full_battery_models/test_lithium_ion/test_mpm.py index eb2a88f880..aa8674acca 100644 --- a/tests/integration/test_models/test_full_battery_models/test_lithium_ion/test_mpm.py +++ b/tests/integration/test_models/test_full_battery_models/test_lithium_ion/test_mpm.py @@ -42,6 +42,12 @@ def test_particle_uniform(self): modeltest = tests.StandardModelTest(model) modeltest.test_all() + def test_differential_surface_form(self): + options = {"surface form": "differential"} + model = pybamm.lithium_ion.MPM(options) + modeltest = tests.StandardModelTest(model) + modeltest.test_all() + def test_voltage_control(self): options = {"operating mode": "voltage"} model = pybamm.lithium_ion.MPM(options) 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 a80b7053b9..4e78727e4e 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 @@ -55,11 +55,20 @@ def test_particle_quadratic(self): with self.assertRaises(NotImplementedError): pybamm.lithium_ion.MPM(options) + def test_differential_surface_form(self): + options = {"surface form": "differential"} + model = pybamm.lithium_ion.MPM(options) + model.check_well_posedness() + def test_necessary_options(self): options = {"particle size": "single"} with self.assertRaises(pybamm.OptionError): pybamm.lithium_ion.MPM(options) + options = {"surface form": "false"} + with self.assertRaises(pybamm.OptionError): + pybamm.lithium_ion.MPM(options) + def test_nonspherical_particle_not_implemented(self): options = {"particle shape": "user"} with self.assertRaises(NotImplementedError): From 1c87d5fced7503c75bbcaff60ae004584000a168 Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Thu, 8 Dec 2022 10:06:50 +0000 Subject: [PATCH 3/4] style: pre-commit fixes --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index a361238e93..2c992f1c0f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,7 @@ # [Unreleased](https://github.com/pybamm-team/PyBaMM/) ## Features + - Allow the option "surface form" to be "differential" in the `MPM` ([#2533](https://github.com/pybamm-team/PyBaMM/pull/2533)) ## Bug fixes From 35aefd239bcb2e2a35e4bd8e1c3aa14e2f7905c0 Mon Sep 17 00:00:00 2001 From: Robert Timms Date: Thu, 8 Dec 2022 11:07:09 +0000 Subject: [PATCH 4/4] MPM w/capacitance fix options --- pybamm/models/full_battery_models/lithium_ion/mpm.py | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/pybamm/models/full_battery_models/lithium_ion/mpm.py b/pybamm/models/full_battery_models/lithium_ion/mpm.py index 1e68b718fa..5a171b3f11 100644 --- a/pybamm/models/full_battery_models/lithium_ion/mpm.py +++ b/pybamm/models/full_battery_models/lithium_ion/mpm.py @@ -41,10 +41,8 @@ class MPM(SPM): def __init__(self, options=None, name="Many-Particle Model", build=True): # Necessary/default options - default_options = {"particle size": "distribution", "surface form": "algebraic"} - if options is None: - options = default_options - elif "particle size" in options and options["particle size"] != "distribution": + options = options or {} + if "particle size" in options and options["particle size"] != "distribution": raise pybamm.OptionError( "particle size must be 'distribution' for MPM not '{}'".format( options["particle size"] @@ -55,7 +53,10 @@ def __init__(self, options=None, name="Many-Particle Model", build=True): "surface form must be 'algebraic' or 'differential' for MPM not 'false'" ) else: - options.update(default_options) + surface_form = options.get("surface form", "algebraic") + options.update( + {"particle size": "distribution", "surface form": surface_form} + ) super().__init__(options, name, build) pybamm.citations.register("Kirk2020")