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.

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

Issue 1582 concentration #1866

Merged
merged 61 commits into from
Feb 2, 2022
Merged
Show file tree
Hide file tree
Changes from 55 commits
Commits
Show all changes
61 commits
Select commit Hold shift + click to select a range
18ac674
#1582 allow initial concentration to depend on r
valentinsulzer Aug 5, 2021
dc0dd72
#1582 flake8
valentinsulzer Aug 6, 2021
1bee8cc
Merge branch 'develop' into issue-1582-concentration
valentinsulzer Aug 6, 2021
c6daef5
#1582 fix tests
valentinsulzer Aug 6, 2021
611219f
#1582 merge develop
valentinsulzer Aug 9, 2021
cbfdbea
#1582 merge develop
valentinsulzer Sep 17, 2021
a7eb9b9
Merge branch 'develop' into issue-1582-concentration
valentinsulzer Nov 4, 2021
eb2042b
#1582 fixing tests
valentinsulzer Nov 5, 2021
569f814
Merge branch 'develop' into issue-1582-concentration
valentinsulzer Nov 9, 2021
de6bce4
#1582 merge develop
valentinsulzer Nov 26, 2021
4e046fe
Merge branch 'issue-1816-domain-error' into issue-1582-concentration
valentinsulzer Nov 26, 2021
b69d1d0
#1582 fix compare_lithium_ion
valentinsulzer Nov 26, 2021
8e79d8c
#1582 merge #1781
valentinsulzer Nov 26, 2021
d6b08cb
#1582 fix tests
valentinsulzer Nov 27, 2021
2ae4d5c
#1582 merge averages-class
valentinsulzer Dec 5, 2021
8ea7dc7
#1582 merge develop
valentinsulzer Dec 14, 2021
9d8cd09
Merge branch 'develop' into issue-1582-concentration
valentinsulzer Dec 19, 2021
1d86579
#1582 fix some tests
valentinsulzer Dec 19, 2021
2bda121
#1582 remove accidental duplicate files
valentinsulzer Dec 19, 2021
d00ffb5
#1582 reduce if statements
valentinsulzer Dec 19, 2021
40e2681
#1582 revert examples
valentinsulzer Dec 20, 2021
038d3f9
#1582 fix test
valentinsulzer Dec 21, 2021
c3ffef2
merge develop
valentinsulzer Dec 21, 2021
647540a
#1582 fixing tests
valentinsulzer Dec 29, 2021
556490d
Merge branch 'develop' into issue-1582-concentration
valentinsulzer Dec 31, 2021
cdde52a
flake8
valentinsulzer Dec 31, 2021
be7f4a8
#1582 fix dfn tests
valentinsulzer Jan 3, 2022
966f3a3
#1582 revert examples
valentinsulzer Jan 3, 2022
ab056d2
#1582 fix test
valentinsulzer Jan 3, 2022
1040d99
#1582 fix example
valentinsulzer Jan 4, 2022
ca88f73
Merge branch 'switch-coverage' into issue-1582-concentration
valentinsulzer Jan 4, 2022
b12b244
#1582 coverage
valentinsulzer Jan 5, 2022
f7ab75f
#1582 fix test
valentinsulzer Jan 5, 2022
9931aaf
#1582 flake8
valentinsulzer Jan 5, 2022
7683807
Merge branch 'switch-coverage' into issue-1582-concentration
valentinsulzer Jan 5, 2022
d8ed612
Merge branch 'develop' into issue-1582-concentration
valentinsulzer Jan 11, 2022
ba5f2aa
#1582 coverage
valentinsulzer Jan 11, 2022
053d5cd
#1582 replace some auxiliary_domains with domains
valentinsulzer Jan 11, 2022
d4d044a
#1582 replacing auxiliary_domains with domains
valentinsulzer Jan 12, 2022
c36c0b4
merge develop
valentinsulzer Jan 17, 2022
7b8d60b
#1582 fix tests except save/load
valentinsulzer Jan 17, 2022
6d4371a
remove gif
valentinsulzer Jan 17, 2022
6c8d7aa
#1582 fix save/load
valentinsulzer Jan 17, 2022
20e6f41
#1582 benchmark test
valentinsulzer Jan 17, 2022
f6ce7bd
#1582 more tests and benchmarks
valentinsulzer Jan 18, 2022
31a30fb
#1582 flake8
valentinsulzer Jan 19, 2022
8fda417
#1582 fix tests
valentinsulzer Jan 20, 2022
a2cb799
#1582 fix docs
valentinsulzer Jan 20, 2022
13e25c8
#1582 flake8
valentinsulzer Jan 20, 2022
5eb20f7
#1582 merge develop
valentinsulzer Jan 20, 2022
434a544
#158 coverage
valentinsulzer Jan 20, 2022
a050d8d
#1582 coverage and benchmarks
valentinsulzer Jan 20, 2022
d0af13b
Merge branch 'develop' into issue-1582-concentration
valentinsulzer Jan 21, 2022
ba9a4d0
#1582 merge develop
valentinsulzer Jan 23, 2022
79e330d
#1582 test for benchmarks
valentinsulzer Jan 24, 2022
9b0aa46
Merge branch 'develop' into issue-1582-concentration
valentinsulzer Feb 1, 2022
f1a644a
#1582 test benchmarks
valentinsulzer Feb 1, 2022
a9acb6b
#1582 more benchmarking
valentinsulzer Feb 1, 2022
7cc0d9f
#1582 deprecate domain setter and fix tests
valentinsulzer Feb 2, 2022
6f636bd
#1582 changelog
valentinsulzer Feb 2, 2022
21b84ba
#1582 remove test.json
valentinsulzer Feb 2, 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
2 changes: 1 addition & 1 deletion pybamm/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@
#
# Utility classes and methods
#
from .util import Timer, TimerTime, FuzzyDict
from .util import Timer, TimerTime, FuzzyDict, DomainDict
from .util import root_dir, load_function, rmse, get_infinite_nested_dict, load
from .util import get_parameters_filepath, have_jax, install_jax, is_jax_compatible
from .logger import logger, set_logging_level
Expand Down
42 changes: 19 additions & 23 deletions pybamm/discretisations/discretisation.py
Original file line number Diff line number Diff line change
Expand Up @@ -236,6 +236,16 @@ def process_model(self, model, inplace=True, check_model=True):
model_disc
)

# Process length scales
new_length_scales = {}
for domain, scale in model.length_scales.items():
new_scale = self.process_symbol(scale)
if isinstance(new_scale, pybamm.Array):
# Convert possible arrays of length 1 to scalars
new_scale = pybamm.Scalar(float(new_scale.evaluate()))
new_length_scales[domain] = new_scale
model_disc._length_scales = new_length_scales

# Check that resulting model makes sense
if check_model:
pybamm.logger.verbose("Performing model checks for {}".format(model.name))
Expand Down Expand Up @@ -317,9 +327,7 @@ def _get_variable_size(self, variable):
else:
size = 0
spatial_method = self.spatial_methods[variable.domain[0]]
repeats = spatial_method._get_auxiliary_domain_repeats(
variable.auxiliary_domains
)
repeats = spatial_method._get_auxiliary_domain_repeats(variable.domains)
for dom in variable.domain:
size += spatial_method.mesh[dom].npts_for_broadcast_to_nodes * repeats
return size
Expand Down Expand Up @@ -782,9 +790,7 @@ def process_dict(self, var_eqn_dict):
for eqn_key, eqn in var_eqn_dict.items():
# Broadcast if the equation evaluates to a number (e.g. Scalar)
if np.prod(eqn.shape_for_testing) == 1 and not isinstance(eqn_key, str):
eqn = pybamm.FullBroadcast(
eqn, eqn_key.domain, eqn_key.auxiliary_domains
)
eqn = pybamm.FullBroadcast(eqn, broadcast_domains=eqn_key.domains)

# note we are sending in the key.id here so we don't have to
# keep calling .id
Expand Down Expand Up @@ -824,9 +830,9 @@ def process_symbol(self, symbol):
else:
discretised_symbol.mesh = None
# Assign secondary mesh
if "secondary" in symbol.auxiliary_domains:
if symbol.domains["secondary"] != []:
discretised_symbol.secondary_mesh = self.mesh.combine_submeshes(
*symbol.auxiliary_domains["secondary"]
*symbol.domains["secondary"]
)
else:
discretised_symbol.secondary_mesh = None
Expand Down Expand Up @@ -937,10 +943,7 @@ def _process_symbol(self, symbol):
out = disc_child * pybamm.Vector([1])
else:
out = spatial_method.broadcast(
disc_child,
symbol.domain,
symbol.auxiliary_domains,
symbol.broadcast_type,
disc_child, symbol.domains, symbol.broadcast_type
)
return out

Expand Down Expand Up @@ -976,8 +979,7 @@ def _process_symbol(self, symbol):
elif isinstance(symbol, pybamm.VariableDot):
return pybamm.StateVectorDot(
*self.y_slices[symbol.get_variable().id],
domain=symbol.domain,
auxiliary_domains=symbol.auxiliary_domains,
domains=symbol.domains,
)

elif isinstance(symbol, pybamm.Variable):
Expand All @@ -992,8 +994,7 @@ def _process_symbol(self, symbol):
return pybamm.ExternalVariable(
symbol.name,
size=self._get_variable_size(symbol),
domain=symbol.domain,
auxiliary_domains=symbol.auxiliary_domains,
domains=symbol.domains,
)
else:
# We have to use a special name since the concatenation doesn't have
Expand All @@ -1002,8 +1003,7 @@ def _process_symbol(self, symbol):
ext = pybamm.ExternalVariable(
name,
size=self._get_variable_size(parent),
domain=parent.domain,
auxiliary_domains=parent.auxiliary_domains,
domains=parent.domains,
)
out = pybamm.Index(ext, slice(start, end))
out.domain = symbol.domain
Expand All @@ -1025,11 +1025,7 @@ def _process_symbol(self, symbol):
symbol.name
)
)
return pybamm.StateVector(
*y_slices,
domain=symbol.domain,
auxiliary_domains=symbol.auxiliary_domains,
)
return pybamm.StateVector(*y_slices, domains=symbol.domains)

elif isinstance(symbol, pybamm.SpatialVariable):
return spatial_method.spatial_variable(symbol)
Expand Down
15 changes: 11 additions & 4 deletions pybamm/expression_tree/array.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,11 @@ class Array(pybamm.Symbol):
list of domains the parameter is valid over, defaults to empty list
auxiliary_domainds : dict, optional
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wasn't you, but there is a typo in auxiliary_domains

dictionary of auxiliary domains, defaults to empty dict
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.
entries_string : str
String representing the entries (slow to recalculate when copying)

Expand All @@ -37,6 +42,7 @@ def __init__(
name=None,
domain=None,
auxiliary_domains=None,
domains=None,
entries_string=None,
):
# if
Expand All @@ -49,7 +55,9 @@ def __init__(
self._entries = entries.astype(float)
# Use known entries string to avoid re-hashing, where possible
self.entries_string = entries_string
super().__init__(name, domain=domain, auxiliary_domains=auxiliary_domains)
super().__init__(
name, domain=domain, auxiliary_domains=auxiliary_domains, domains=domains
)

@property
def entries(self):
Expand Down Expand Up @@ -105,9 +113,8 @@ def create_copy(self):
return self.__class__(
self.entries,
self.name,
self.domain,
self.auxiliary_domains,
self.entries_string,
domains=self.domains,
entries_string=self.entries_string,
)

def _base_evaluate(self, t=None, y=None, y_dot=None, inputs=None):
Expand Down
74 changes: 32 additions & 42 deletions pybamm/expression_tree/averages.py
Original file line number Diff line number Diff line change
Expand Up @@ -75,12 +75,7 @@ def _unary_new_copy(self, child):

class SizeAverage(_BaseAverage):
def __init__(self, child, f_a_dist):
R = pybamm.SpatialVariable(
"R",
domain=child.domain,
auxiliary_domains=child.auxiliary_domains,
coord_sys="cartesian",
)
R = pybamm.SpatialVariable("R", domains=child.domains, coord_sys="cartesian")
integration_variable = [R]
super().__init__(child, "size-average", integration_variable)
self.f_a_dist = f_a_dist
Expand Down Expand Up @@ -134,22 +129,22 @@ def x_average(symbol):
dom in ["negative electrode", "separator", "positive electrode"]
for dom in symbol.secondary_domain
):
aux = {}
if "tertiary" in symbol.auxiliary_domains:
aux["secondary"] = symbol.auxiliary_domains["tertiary"]
return pybamm.FullBroadcast(symbol.orphans[0], symbol.broadcast_domain, aux)
elif (
isinstance(symbol, pybamm.FullBroadcast)
and "tertiary" in symbol.auxiliary_domains
and all(
dom in ["negative electrode", "separator", "positive electrode"]
for dom in symbol.tertiary_domain
)
domains = {
"primary": symbol.domains["primary"],
"secondary": symbol.domains["tertiary"],
"tertiary": symbol.domains["quaternary"],
}
return pybamm.FullBroadcast(symbol.orphans[0], broadcast_domains=domains)
elif isinstance(symbol, pybamm.FullBroadcast) and all(
dom in ["negative electrode", "separator", "positive electrode"]
for dom in symbol.tertiary_domain
):
aux = {"secondary": symbol.auxiliary_domains["secondary"]}
if "quaternary" in symbol.auxiliary_domains:
aux["tertiary"] = symbol.auxiliary_domains["quaternary"]
return pybamm.FullBroadcast(symbol.orphans[0], symbol.broadcast_domain, aux)
domains = {
"primary": symbol.domains["primary"],
"secondary": symbol.domains["secondary"],
"tertiary": symbol.domains["quaternary"],
}
return pybamm.FullBroadcast(symbol.orphans[0], broadcast_domains=domains)
else: # pragma: no cover
# It should be impossible to get here
raise NotImplementedError
Expand All @@ -174,14 +169,14 @@ def x_average(symbol):
if out.domain != []:
return out
# Otherwise we may need to broadcast it
elif child.auxiliary_domains == {}:
elif child.domains["secondary"] == []:
return out
else:
domain = child.auxiliary_domains["secondary"]
if "tertiary" not in child.auxiliary_domains:
domain = child.domains["secondary"]
if child.domains["tertiary"] == []:
return pybamm.PrimaryBroadcast(out, domain)
else:
auxiliary_domains = {"secondary": child.auxiliary_domains["tertiary"]}
auxiliary_domains = {"secondary": child.domains["tertiary"]}
return pybamm.FullBroadcast(out, domain, auxiliary_domains)
# Otherwise, use Integral to calculate average value
else:
Expand Down Expand Up @@ -218,7 +213,7 @@ def z_average(symbol):
return symbol
# If symbol is a Broadcast, its average value is its child
elif isinstance(symbol, pybamm.Broadcast):
return symbol.orphans[0]
return symbol.reduce_one_dimension()
# Otherwise, define a ZAverage
else:
return ZAverage(symbol)
Expand Down Expand Up @@ -251,12 +246,16 @@ def yz_average(symbol):
return symbol
# If symbol is a Broadcast, its average value is its child
elif isinstance(symbol, pybamm.Broadcast):
return symbol.orphans[0]
return symbol.reduce_one_dimension()
# Otherwise, define a YZAverage
else:
return YZAverage(symbol)


def xyz_average(symbol):
return yz_average(x_average(symbol))


def r_average(symbol):
"""
Convenience function for creating an average in the r-direction.
Expand All @@ -276,11 +275,7 @@ def r_average(symbol):
raise ValueError("Can't take the r-average of a symbol that evaluates on edges")
# Otherwise, if symbol doesn't have a particle domain,
# its r-averaged value is itself
elif symbol.domain not in [
["positive particle"],
["negative particle"],
["working particle"],
]:
elif symbol.domain not in [["positive particle"], ["negative particle"]]:
return symbol
# If symbol is a secondary broadcast onto "negative electrode" or
# "positive electrode", take the r-average of the child then broadcast back
Expand All @@ -291,12 +286,10 @@ def r_average(symbol):
child_av = pybamm.r_average(child)
return pybamm.PrimaryBroadcast(child_av, symbol.domains["secondary"])
# If symbol is a Broadcast onto a particle domain, its average value is its child
elif isinstance(symbol, pybamm.PrimaryBroadcast) and symbol.domain in [
["positive particle"],
["negative particle"],
["working particle"],
]:
return symbol.orphans[0]
elif isinstance(
symbol, (pybamm.PrimaryBroadcast, pybamm.FullBroadcast)
) and symbol.domain in [["positive particle"], ["negative particle"]]:
return symbol.reduce_one_dimension()
else:
return RAverage(symbol)

Expand Down Expand Up @@ -345,10 +338,7 @@ def size_average(symbol, f_a_dist=None):
if f_a_dist is None:
geo = pybamm.geometric_parameters
R = pybamm.SpatialVariable(
"R",
domain=symbol.domain,
auxiliary_domains=symbol.auxiliary_domains,
coord_sys="cartesian",
"R", domains=symbol.domains, coord_sys="cartesian"
)
if ["negative particle size"] in symbol.domains.values():
f_a_dist = geo.f_a_dist_n(R)
Expand Down
54 changes: 10 additions & 44 deletions pybamm/expression_tree/binary_operators.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,42 +25,15 @@ def preprocess_binary(left, right):
)

# Do some broadcasting in special cases, to avoid having to do this manually
if left.domain != [] and right.domain != []:
if (
left.domain != right.domain
and "secondary" in right.auxiliary_domains
and left.domain == right.auxiliary_domains["secondary"]
):
if left.domain != [] and right.domain != [] and left.domain != right.domain:
if left.domain == right.secondary_domain:
left = pybamm.PrimaryBroadcast(left, right.domain)
if (
right.domain != left.domain
and "secondary" in left.auxiliary_domains
and right.domain == left.auxiliary_domains["secondary"]
):
elif right.domain == left.secondary_domain:
right = pybamm.PrimaryBroadcast(right, left.domain)

return left, right


def get_binary_children_domains(ldomain, rdomain):
"""Combine domains from children in appropriate way."""
if ldomain == rdomain:
return ldomain
elif ldomain == []:
return rdomain
elif rdomain == []:
return ldomain
else:
raise pybamm.DomainError(
"""
children must have same (or empty) domains, but left.domain is '{}'
and right.domain is '{}'
""".format(
ldomain, rdomain
)
)


class BinaryOperator(pybamm.Symbol):
"""
A node in the expression tree representing a binary operator (e.g. `+`, `*`)
Expand All @@ -83,14 +56,8 @@ class BinaryOperator(pybamm.Symbol):
def __init__(self, name, left, right):
left, right = preprocess_binary(left, right)

domain = get_binary_children_domains(left.domain, right.domain)
auxiliary_domains = self.get_children_auxiliary_domains([left, right])
super().__init__(
name,
children=[left, right],
domain=domain,
auxiliary_domains=auxiliary_domains,
)
domains = self.get_children_domains([left, right])
super().__init__(name, children=[left, right], domains=domains)
self.left = self.children[0]
self.right = self.children[1]

Expand Down Expand Up @@ -413,12 +380,7 @@ def _binary_evaluate(self, left, right):
if issparse(left):
return csr_matrix(left.multiply(1 / right))
else:
if isinstance(right, numbers.Number) and right == 0:
# don't raise RuntimeWarning for NaNs
with np.errstate(invalid="ignore"):
return left * np.inf
else:
return left / right
return left / right


class Inner(BinaryOperator):
Expand Down Expand Up @@ -988,6 +950,10 @@ def simplified_subtraction(left, right):
):
return left

# Return constant if both sides are constant
if left.is_constant() and right.is_constant():
return pybamm.simplify_if_constant(Subtraction(left, right))

# a symbol minus itself is 0s of the same shape
if left.id == right.id:
return pybamm.zeros_like(left)
Expand Down
Loading