merge develop, back to long form for lam

Author: valentinsulzer

CHANGELOG.md

@@ -2,7 +2,7 @@
 
 ## Features
 
-- Added variables "Loss of lithium due to LAM in negative/positive electrode [mol]", and "Total lithium lost to LAM [mol]/[A.h]". This is also included in the calculation of "total lithium in system" to make sure that lithium is truly conserved. ([#2529](https://github.com/pybamm-team/PyBaMM/pull/2529))
+- Added variables "Loss of lithium due to loss of active material in negative/positive electrode [mol]". These should be included in the calculation of "total lithium in system" to make sure that lithium is truly conserved. ([#2529](https://github.com/pybamm-team/PyBaMM/pull/2529))
 - `initial_soc` can now be a string "x V", in which case the simulation is initialized to start from that voltage ([#2508](https://github.com/pybamm-team/PyBaMM/pull/2508))
 - The `ElectrodeSOH` solver can now calculate electrode balance based on a target "cell capacity" (requires cell capacity "Q" as input), as well as the default "cyclable cell capacity" (requires cyclable lithium capacity "Q_Li" as input). Use the keyword argument `known_value` to control which is used. ([#2508](https://github.com/pybamm-team/PyBaMM/pull/2508))
 
@@ -14,6 +14,7 @@
 ## Breaking changes
 
 - Renamed "Negative/Positive electrode SOC" to "Negative/Positive electrode stoichiometry" to avoid confusion with cell SOC ([#2529](https://github.com/pybamm-team/PyBaMM/pull/2529))
+- Removed external variables and submodels. InputParameter should now be used in all cases ([#2502](https://github.com/pybamm-team/PyBaMM/pull/2502))
 - Trying to use a solver to solve multiple models results in a RuntimeError exception ([#2481](https://github.com/pybamm-team/PyBaMM/pull/2481))
 - Inputs for the `ElectrodeSOH` solver are now (i) "Q_Li", the total cyclable capacity of lithium in the electrodes (previously "n_Li", the total number of moles, n_Li = 3600/F \* Q_Li) (ii) "Q_n", the capacity of the negative electrode (previously "C_n"), and "Q_p", the capacity of the positive electrode (previously "C_p") ([#2508](https://github.com/pybamm-team/PyBaMM/pull/2508))
 

docs/source/expression_tree/variable.rst

@@ -7,6 +7,3 @@ Variable
 .. autoclass:: pybamm.VariableDot
   :members:
 
-.. autoclass:: pybamm.ExternalVariable
-  :members:
-

pybamm/discretisations/discretisation.py

@@ -63,7 +63,6 @@ def __init__(self, mesh=None, spatial_methods=None):
         self._bcs = {}
         self.y_slices = {}
         self._discretised_symbols = {}
-        self.external_variables = {}
 
     @property
     def mesh(self):
@@ -178,11 +177,6 @@ def process_model(
         pybamm.logger.verbose("Set variable slices for {}".format(model.name))
         self.set_variable_slices(variables)
 
-        # now add extrapolated external variables to the boundary conditions
-        # if required by the spatial method
-        self._preprocess_external_variables(model)
-        self.set_external_variables(model)
-
         # set boundary conditions (only need key ids for boundary_conditions)
         pybamm.logger.verbose(
             "Discretise boundary conditions for {}".format(model.name)
@@ -242,11 +236,6 @@ def process_model(
             processed_events.append(processed_event)
         model_disc.events = processed_events
 
-        # Set external variables
-        model_disc.external_variables = [
-            self.process_symbol(var) for var in model.external_variables
-        ]
-
         # Create mass matrix
         pybamm.logger.verbose("Create mass matrix for {}".format(model.name))
         model_disc.mass_matrix, model_disc.mass_matrix_inv = self.create_mass_matrix(
@@ -349,70 +338,6 @@ def _get_variable_size(self, variable):
                 size += spatial_method.mesh[dom].npts_for_broadcast_to_nodes * repeats
             return size
 
-    def _preprocess_external_variables(self, model):
-        """
-        A method to preprocess external variables so that they are
-        compatible with the spatial method. For example, in finite
-        volume, the user will supply a vector of values valid on the
-        cell centres. However, for model processing, we also require
-        the boundary edge fluxes. Therefore, we extrapolate and add
-        the boundary fluxes to the boundary conditions, which are
-        employed in generating the grad and div matrices.
-        The processing is delegated to spatial methods as
-        the preprocessing required for finite volume and finite
-        element will be different.
-        """
-
-        for var in model.external_variables:
-            if var.domain != []:
-                new_bcs = self.spatial_methods[
-                    var.domain[0]
-                ].preprocess_external_variables(var)
-
-                model.boundary_conditions.update(new_bcs)
-
-    def set_external_variables(self, model):
-        """
-        Add external variables to the list of variables to account for, being careful
-        about concatenations
-        """
-        for var in model.external_variables:
-            # Find the name in the model variables
-            # Look up dictionary key based on value
-            try:
-                idx = list(model.variables.values()).index(var)
-            except ValueError:
-                raise ValueError(
-                    """
-                    Variable {} must be in the model.variables dictionary to be set
-                    as an external variable
-                    """.format(
-                        var
-                    )
-                )
-            name = list(model.variables.keys())[idx]
-            if isinstance(var, pybamm.Variable):
-                # No need to keep track of the parent
-                self.external_variables[(name, None)] = var
-            elif isinstance(var, pybamm.ConcatenationVariable):
-                start = 0
-                end = 0
-                for child in var.children:
-                    dom = child.domain[0]
-                    if (
-                        self.spatial_methods[dom]._get_auxiliary_domain_repeats(
-                            child.domains
-                        )
-                        > 1
-                    ):
-                        raise NotImplementedError(
-                            "Cannot create 2D external variable with concatenations"
-                        )
-                    end += self._get_variable_size(child)
-                    # Keep a record of the parent
-                    self.external_variables[(name, (var, start, end))] = child
-                    start = end
-
     def set_internal_boundary_conditions(self, model):
         """
         A method to set the internal boundary conditions for the submodel.
@@ -526,16 +451,15 @@ def process_boundary_conditions(self, model):
 
             # check if the boundary condition at the origin for sphere domains is other
             # than no flux
-            if key not in model.external_variables:
-                for subdomain in key.domain:
-                    if self.mesh[subdomain].coord_sys == "spherical polar":
-                        if bcs["left"][0].value != 0 or bcs["left"][1] != "Neumann":
-                            raise pybamm.ModelError(
-                                "Boundary condition at r = 0 must be a homogeneous "
-                                "Neumann condition for {} coordinates".format(
-                                    self.mesh[subdomain].coord_sys
-                                )
+            for subdomain in key.domain:
+                if self.mesh[subdomain].coord_sys == "spherical polar":
+                    if bcs["left"][0].value != 0 or bcs["left"][1] != "Neumann":
+                        raise pybamm.ModelError(
+                            "Boundary condition at r = 0 must be a homogeneous "
+                            "Neumann condition for {} coordinates".format(
+                                self.mesh[subdomain].coord_sys
                             )
+                        )
 
             # Handle any boundary conditions applied on the tabs
             if any("tab" in side for side in list(bcs.keys())):
@@ -956,52 +880,26 @@ def _process_symbol(self, symbol):
             )
 
         elif isinstance(symbol, pybamm.Variable):
-            # Check if variable is a standard variable or an external variable
-            if any(symbol == var for var in self.external_variables.values()):
-                # Look up dictionary key based on value
-                idx = list(self.external_variables.values()).index(symbol)
-                name, parent_and_slice = list(self.external_variables.keys())[idx]
-                if parent_and_slice is None:
-                    # Variable didn't come from a concatenation so we can just create a
-                    # normal external variable using the symbol's name
-                    return pybamm.ExternalVariable(
-                        symbol.name,
-                        size=self._get_variable_size(symbol),
-                        domains=symbol.domains,
-                    )
-                else:
-                    # We have to use a special name since the concatenation doesn't have
-                    # a very informative name. Needs improving
-                    parent, start, end = parent_and_slice
-                    ext = pybamm.ExternalVariable(
-                        name,
-                        size=self._get_variable_size(parent),
-                        domains=parent.domains,
-                    )
-                    out = pybamm.Index(ext, slice(start, end))
-                    out.copy_domains(symbol)
-                    return out
-            else:
-                # add a try except block for a more informative error if a variable
-                # can't be found. This should usually be caught earlier by
-                # model.check_well_posedness, but won't be if debug_mode is False
-                try:
-                    y_slices = self.y_slices[symbol]
-                except KeyError:
-                    raise pybamm.ModelError(
-                        """
-                        No key set for variable '{}'. Make sure it is included in either
-                        model.rhs, model.algebraic, or model.external_variables in an
-                        unmodified form (e.g. not Broadcasted)
-                        """.format(
-                            symbol.name
-                        )
+            # add a try except block for a more informative error if a variable
+            # can't be found. This should usually be caught earlier by
+            # model.check_well_posedness, but won't be if debug_mode is False
+            try:
+                y_slices = self.y_slices[symbol]
+            except KeyError:
+                raise pybamm.ModelError(
+                    """
+                    No key set for variable '{}'. Make sure it is included in either
+                    model.rhs or model.algebraic in an unmodified form
+                    (e.g. not Broadcasted)
+                    """.format(
+                        symbol.name
                     )
-                # Add symbol's reference and multiply by the symbol's scale
-                # so that the state vector is of order 1
-                return symbol.reference + symbol.scale * pybamm.StateVector(
-                    *y_slices, domains=symbol.domains
                 )
+            # Add symbol's reference and multiply by the symbol's scale
+            # so that the state vector is of order 1
+            return symbol.reference + symbol.scale * pybamm.StateVector(
+                *y_slices, domains=symbol.domains
+            )
 
         elif isinstance(symbol, pybamm.SpatialVariable):
             return spatial_method.spatial_variable(symbol)
@@ -1083,14 +981,7 @@ def _concatenate_in_order(self, var_eqn_dict, check_complete=False, sparse=False
         if check_complete:
             # Check keys from the given var_eqn_dict against self.y_slices
             unpacked_variables_set = set(unpacked_variables)
-            external_vars = set(self.external_variables.values())
-            for var in self.external_variables.values():
-                child_vars = set(var.children)
-                external_vars = external_vars.union(child_vars)
-            y_slices_with_external_removed = set(self.y_slices.keys()).difference(
-                external_vars
-            )
-            if unpacked_variables_set != y_slices_with_external_removed:
+            if unpacked_variables_set != set(self.y_slices.keys()):
                 given_variable_names = [v.name for v in var_eqn_dict.keys()]
                 raise pybamm.ModelError(
                     "Initial conditions are insufficient. Only "

pybamm/expression_tree/exceptions.py

@@ -56,12 +56,6 @@ class ModelWarning(UserWarning):
     pass
 
 
-class InputError(Exception):
-    """An external variable has been input incorrectly into PyBaMM."""
-
-    pass
-
-
 class DiscretisationError(Exception):
     """A model could not be discretised."""
 

pybamm/expression_tree/operations/convert_to_casadi.py

@@ -55,7 +55,6 @@ def _convert(self, symbol, t, y, y_dot, inputs):
                 pybamm.Array,
                 pybamm.Time,
                 pybamm.InputParameter,
-                pybamm.ExternalVariable,
             ),
         ):
             return casadi.MX(symbol.evaluate(t, y, y_dot, inputs))

pybamm/expression_tree/variable.py

@@ -1,7 +1,6 @@
 #
 # Variable class
 #
-import numbers
 
 import numpy as np
 import sympy
@@ -223,107 +222,3 @@ def diff(self, variable):
             raise pybamm.ModelError("cannot take second time derivative of a Variable")
         else:
             return pybamm.Scalar(0)
-
-
-class ExternalVariable(Variable):
-    """
-    A node in the expression tree representing an external variable variable.
-
-    This node will be discretised by :class:`.Discretisation` and converted
-    to a :class:`.Vector` node.
-
-    Parameters
-    ----------
-
-    name : str
-        name of the node
-    domain : iterable of str
-        list of domains that this variable is valid over
-    auxiliary_domains : dict
-        dictionary of auxiliary domains ({'secondary': ..., 'tertiary': ...,
-        'quaternary': ...}). For example, for the single particle model, the particle
-        concentration would be a Variable with domain 'negative particle' and secondary
-        auxiliary domain 'current collector'. For the DFN, the particle concentration
-        would be a Variable with domain 'negative particle', secondary domain
-        'negative electrode' and tertiary domain 'current collector'
-    domains : dict
-        A dictionary equivalent to {'primary': domain, auxiliary_domains}. Either
-        'domain' and 'auxiliary_domains', or just 'domains', should be provided
-        (not both). In future, the 'domain' and 'auxiliary_domains' arguments may be
-        deprecated.
-    scale : float or :class:`pybamm.Symbol`, optional
-        The scale of the variable, used for scaling the model when solving. The state
-        vector representing this variable will be multiplied by this scale.
-        Default is 1.
-    reference : float or :class:`pybamm.Symbol`, optional
-        The reference value of the variable, used for scaling the model when solving.
-        This value will be added to the state vector representing this variable.
-        Default is 0.
-
-    *Extends:* :class:`pybamm.Variable`
-    """
-
-    def __init__(
-        self,
-        name,
-        size,
-        domain=None,
-        auxiliary_domains=None,
-        domains=None,
-        scale=1,
-        reference=0,
-    ):
-        self._size = size
-        super().__init__(
-            name, domain, auxiliary_domains, domains, scale=scale, reference=reference
-        )
-
-    @property
-    def size(self):
-        return self._size
-
-    def create_copy(self):
-        """See :meth:`pybamm.Symbol.new_copy()`."""
-        return ExternalVariable(self.name, self.size, domains=self.domains)
-
-    def _evaluate_for_shape(self):
-        """See :meth:`pybamm.Symbol.evaluate_for_shape_using_domain()`"""
-        return np.nan * np.ones((self.size, 1))
-
-    def _base_evaluate(self, t=None, y=None, y_dot=None, inputs=None):
-        # inputs should be a dictionary
-        # convert 'None' to empty dictionary for more informative error
-        if inputs is None:
-            inputs = {}
-        if not isinstance(inputs, dict):
-            # if the special input "shape test" is passed, just return 1
-            if inputs == "shape test":
-                return self.evaluate_for_shape()
-            raise TypeError("inputs should be a dictionary")
-        try:
-            out = inputs[self.name]
-            if isinstance(out, numbers.Number) or out.shape[0] == 1:
-                return out * np.ones((self.size, 1))
-            elif out.shape[0] != self.size:
-                raise ValueError(
-                    "External variable input has size {} but should be {}".format(
-                        out.shape[0], self.size
-                    )
-                )
-            else:
-                if isinstance(out, np.ndarray) and out.ndim == 1:
-                    out = out[:, np.newaxis]
-                return out
-        # raise more informative error if can't find name in dict
-        except KeyError:
-            raise KeyError("External variable '{}' not found".format(self.name))
-
-    def diff(self, variable):
-        if variable == self:
-            return pybamm.Scalar(1)
-        elif variable == pybamm.t:
-            raise pybamm.ModelError(
-                "cannot take time derivative of an external variable"
-            )
-        else:
-            return pybamm.Scalar(0)