Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

remove external_variables #2502

Merged
merged 17 commits into from
Dec 13, 2022
Merged

remove external_variables #2502

Show file tree
Hide file tree
Changes from 15 commits
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
9ee941a
remove external_variables
TomTranter Nov 29, 2022
665b4a2
style: pre-commit fixes
pre-commit-ci[bot] Nov 29, 2022
ccba82c
style
TomTranter Nov 29, 2022
b725674
remove external submodel functionality and test on spme
TomTranter Dec 1, 2022
428c7cb
Update thermal test
TomTranter Dec 5, 2022
2e1d559
remove external current collector test for now
TomTranter Dec 5, 2022
26ed995
style
TomTranter Dec 5, 2022
ec68fe1
style: pre-commit fixes
pre-commit-ci[bot] Dec 5, 2022
d6d12cd
remove tests no longer needed
TomTranter Dec 6, 2022
917edca
style: pre-commit fixes
pre-commit-ci[bot] Dec 6, 2022
d5d6bcd
Remove ExternalVariable Class
TomTranter Dec 6, 2022
9aba943
Merge branch 'remove_external_variables' of https://github.com/pybamm…
TomTranter Dec 6, 2022
a10ced0
remove numbers
TomTranter Dec 6, 2022
466a875
remove more tests
TomTranter Dec 13, 2022
c5409f8
Merge branch 'develop' into remove_external_variables
valentinsulzer Dec 13, 2022
3ce5439
flake8 and remove other mentions of external variables/submodels
valentinsulzer Dec 13, 2022
bff1f46
coverage and error message for wrong order
valentinsulzer Dec 13, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 0 additions & 3 deletions docs/source/expression_tree/variable.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,3 @@ Variable
.. autoclass:: pybamm.VariableDot
:members:

.. autoclass:: pybamm.ExternalVariable
:members:

163 changes: 27 additions & 136 deletions pybamm/discretisations/discretisation.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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())):
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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 "
Expand Down
1 change: 0 additions & 1 deletion pybamm/expression_tree/operations/convert_to_casadi.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand Down
105 changes: 0 additions & 105 deletions pybamm/expression_tree/variable.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
#
# Variable class
#
import numbers

import numpy as np
import sympy
Expand Down Expand Up @@ -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)
Loading