Merge pull request #862 from pybamm-team/issue-861-remove-set-external

#861 remove external submodels

Author: valentinsulzer

CHANGELOG.md

@@ -1,3 +1,9 @@
+# [Unreleased](https://github.com/pybamm-team/PyBaMM)
+
+## Breaking changes
+
+-   Removed "set external temperature" and "set external potential" options. Use "external submodels" option instead ([#862](https://github.com/pybamm-team/PyBaMM/pull/862))
+
 # [v0.2.0](https://github.com/pybamm-team/PyBaMM/tree/v0.2.0) - 2020-02-26
 
 This release introduces many new features and optimizations. All models can now be solved using the pip installation - in particular, the DFN can be solved in around 0.1s. Other highlights include an improved user interface, simulations of experimental protocols (GITT, CCCV, etc), new parameter sets for NCA and LGM50, drive cycles, "input parameters" and "external variables" for quickly solving models with different parameter values and coupling with external software, and general bug fixes and optimizations.

docs/source/models/submodels/current_collector/index.rst

@@ -10,4 +10,3 @@ Current Collector
   homogeneous_current_collector
   potential_pair
   quite_conductive_potential_pair
-  set_potential_single_particle

docs/source/models/submodels/current_collector/set_potential_single_particle.rst

@@ -9,4 +9,3 @@ X-lumped
   x_lumped_0D_current_collector
   x_lumped_1D_current_collector
   x_lumped_2D_current_collector
-  x_lumped_1D_set_temperature

docs/source/models/submodels/thermal/x_lumped/index.rst

@@ -37,18 +37,14 @@ class BaseBatteryModel(pybamm.BaseModel):
                 (default) or "varying". Not currently implemented in any of the models.
             * "current collector" : str, optional
                 Sets the current collector model to use. Can be "uniform" (default),
-                "potential pair", "potential pair quite conductive", or
-                "set external potential". The submodel "set external potential" can only
-                be used with the SPM.
+                "potential pair" or "potential pair quite conductive".
             * "particle" : str, optional
                 Sets the submodel to use to describe behaviour within the particle.
                 Can be "Fickian diffusion" (default) or "fast diffusion".
             * "thermal" : str, optional
                 Sets the thermal model to use. Can be "isothermal" (default),
-                "x-full", "x-lumped", "xyz-lumped", "lumped" or "set external
-                temperature". Must be "isothermal" for lead-acid models. If the
-                option "set external temperature" is selected then "dimensionality"
-                must be 1.
+                "x-full", "x-lumped", "xyz-lumped" or "lumped". Must be "isothermal" for
+                lead-acid models.
             * "thermal current collector" : bool, optional
                 Whether to include thermal effects in the current collector in
                 one-dimensional models (default is False). Note that this option
@@ -193,7 +189,6 @@ def options(self, extra_options):
             "uniform",
             "potential pair",
             "potential pair quite conductive",
-            "set external potential",
         ]:
             raise pybamm.OptionError(
                 "current collector model '{}' not recognised".format(
@@ -212,7 +207,6 @@ def options(self, extra_options):
             "x-lumped",
             "xyz-lumped",
             "lumped",
-            "set external temperature",
         ]:
             raise pybamm.OptionError(
                 "Unknown thermal model '{}'".format(options["thermal"])
@@ -241,14 +235,6 @@ def options(self, extra_options):
                 raise pybamm.OptionError(
                     "thermal effects not implemented for lead-acid models"
                 )
-        if options["current collector"] == "set external potential" and not isinstance(
-            self, pybamm.lithium_ion.SPM
-        ):
-            raise pybamm.OptionError(
-                "option {} only compatible with SPM".format(
-                    options["current collector"]
-                )
-            )
 
         self._options = options
 
@@ -544,14 +530,6 @@ def set_thermal_submodel(self):
                     self.param
                 )
 
-        elif self.options["thermal"] == "set external temperature":
-            if self.options["dimensionality"] == 1:
-                thermal_submodel = pybamm.thermal.x_lumped.SetTemperature1D(self.param)
-            elif self.options["dimensionality"] in [0, 2]:
-                raise NotImplementedError(
-                    """Set temperature model only implemented for 1D current
-                    collectors"""
-                )
         self.submodels["thermal"] = thermal_submodel
 
     def set_current_collector_submodel(self):
@@ -563,20 +541,6 @@ def set_current_collector_submodel(self):
                 submodel = pybamm.current_collector.PotentialPair1plus1D(self.param)
             elif self.options["dimensionality"] == 2:
                 submodel = pybamm.current_collector.PotentialPair2plus1D(self.param)
-        elif self.options["current collector"] == "set external potential":
-            if self.options["dimensionality"] == 1:
-                submodel = pybamm.current_collector.SetPotentialSingleParticle1plus1D(
-                    self.param
-                )
-            elif self.options["dimensionality"] == 2:
-                submodel = pybamm.current_collector.SetPotentialSingleParticle2plus1D(
-                    self.param
-                )
-            elif self.options["dimensionality"] == 0:
-                raise NotImplementedError(
-                    """Set potential model only implemented for 1D or 2D current
-                    collectors"""
-                )
         self.submodels["current collector"] = submodel
 
     def set_voltage_variables(self):

docs/source/models/submodels/thermal/x_lumped/x_lumped_1D_set_temperature.rst

@@ -17,8 +17,3 @@
     QuiteConductivePotentialPair1plus1D,
     QuiteConductivePotentialPair2plus1D,
 )
-from .set_potential_single_particle import (
-    BaseSetPotentialSingleParticle,
-    SetPotentialSingleParticle1plus1D,
-    SetPotentialSingleParticle2plus1D,
-)

pybamm/models/full_battery_models/base_battery_model.py

@@ -3,4 +3,3 @@
 from .x_lumped_0D_current_collectors import CurrentCollector0D
 from .x_lumped_1D_current_collectors import CurrentCollector1D
 from .x_lumped_2D_current_collectors import CurrentCollector2D
-from .x_lumped_1D_set_temperature import SetTemperature1D

pybamm/models/submodels/current_collector/__init__.py

@@ -122,8 +122,6 @@ def test_bad_options(self):
             pybamm.BaseBatteryModel({"surface form": "bad surface form"})
         with self.assertRaisesRegex(pybamm.OptionError, "particle model"):
             pybamm.BaseBatteryModel({"particle": "bad particle"})
-        with self.assertRaisesRegex(pybamm.OptionError, "option set external"):
-            pybamm.BaseBatteryModel({"current collector": "set external potential"})
         with self.assertRaisesRegex(pybamm.OptionError, "operating mode"):
             pybamm.BaseBatteryModel({"operating mode": "bad operating mode"})
 

pybamm/models/submodels/current_collector/set_potential_single_particle.py

@@ -142,23 +142,6 @@ def test_x_lumped_thermal_2D_current_collector(self):
         model = pybamm.lithium_ion.DFN(options)
         model.check_well_posedness()
 
-    def test_x_lumped_thermal_set_temperature_1D(self):
-        options = {
-            "current collector": "potential pair",
-            "dimensionality": 1,
-            "thermal": "set external temperature",
-        }
-        model = pybamm.lithium_ion.DFN(options)
-        model.check_well_posedness()
-
-        options = {
-            "current collector": "potential pair",
-            "dimensionality": 2,
-            "thermal": "set external temperature",
-        }
-        with self.assertRaises(NotImplementedError):
-            model = pybamm.lithium_ion.DFN(options)
-
     def test_particle_fast_diffusion(self):
         options = {"particle": "fast diffusion"}
         model = pybamm.lithium_ion.DFN(options)

pybamm/models/submodels/thermal/x_lumped/__init__.py

@@ -39,18 +39,6 @@ def test_well_posed_2plus1D(self):
         model = pybamm.lithium_ion.SPM(options)
         model.check_well_posedness()
 
-        options = {"current collector": "set external potential", "dimensionality": 0}
-        with self.assertRaises(NotImplementedError):
-            pybamm.lithium_ion.SPM(options)
-
-        options = {"current collector": "set external potential", "dimensionality": 1}
-        model = pybamm.lithium_ion.SPM(options)
-        model.check_well_posedness()
-
-        options = {"current collector": "set external potential", "dimensionality": 2}
-        model = pybamm.lithium_ion.SPM(options)
-        model.check_well_posedness()
-
     def test_x_full_thermal_model_no_current_collector(self):
         options = {"thermal": "x-full"}
         model = pybamm.lithium_ion.SPM(options)
@@ -155,23 +143,6 @@ def test_x_lumped_thermal_2D_current_collector(self):
         model = pybamm.lithium_ion.SPM(options)
         model.check_well_posedness()
 
-    def test_x_lumped_thermal_set_temperature_1D(self):
-        options = {
-            "current collector": "potential pair",
-            "dimensionality": 1,
-            "thermal": "set external temperature",
-        }
-        model = pybamm.lithium_ion.SPM(options)
-        model.check_well_posedness()
-
-        options = {
-            "current collector": "potential pair",
-            "dimensionality": 2,
-            "thermal": "set external temperature",
-        }
-        with self.assertRaises(NotImplementedError):
-            model = pybamm.lithium_ion.SPM(options)
-
     def test_particle_fast_diffusion(self):
         options = {"particle": "fast diffusion"}
         model = pybamm.lithium_ion.SPM(options)