Skip to content

Commit

Permalink
#759 fix tests, add some doctrings and fix flake8
Browse files Browse the repository at this point in the history
  • Loading branch information
martinjrobins committed Feb 6, 2020
1 parent 50ca0fc commit d844228
Show file tree
Hide file tree
Showing 7 changed files with 62 additions and 51 deletions.
6 changes: 3 additions & 3 deletions pybamm/discretisations/discretisation.py
Original file line number Diff line number Diff line change
Expand Up @@ -178,9 +178,9 @@ def process_model(self, model, inplace=True, check_model=True):
for event in model.events:
pybamm.logger.debug("Discretise event '{}'".format(event.name))
processed_event = pybamm.Event(
event.name,
self.process_symbol(event.expression),
event.event_type
event.name,
self.process_symbol(event.expression),
event.event_type
)
processed_events.append(processed_event)
model_disc.events = processed_events
Expand Down
16 changes: 12 additions & 4 deletions pybamm/models/event.py
Original file line number Diff line number Diff line change
@@ -1,10 +1,18 @@
import pybamm

from enum import Enum


class EventType(Enum):
"""Defines the type of event, see Event"""
"""
Defines the type of event, see Event
TERMINATION indicates an event that will terminate the solver, the expression should
return 0 when the event is triggered
DISCONTINUITY indicates an expected discontinuity in the solution, the expression
should return the time that the discontinuity occurs. The solver will integrate up
to the discontinuity and then restart just after the discontinuity.
"""
TERMINATION = 0
DISCONTINUITY = 1

Expand All @@ -20,7 +28,7 @@ class Event:
name: str
A string giving the name of the event
event_type: EventType
An enum defining the type of event, see EventType
An enum defining the type of event
expression: pybamm.Symbol
An expression that defines when the event occurs
Expand Down
33 changes: 16 additions & 17 deletions pybamm/models/full_battery_models/lithium_ion/basic_dfn.py
Original file line number Diff line number Diff line change
Expand Up @@ -180,20 +180,16 @@ def __init__(self, name="Doyle-Fuller-Newman model"):
self.initial_conditions[c_s_n] = param.c_n_init
self.initial_conditions[c_s_p] = param.c_p_init
# Events specify points at which a solution should terminate
self.events.update(
{
"Minimum negative particle surface concentration": (
pybamm.min(c_s_surf_n) - 0.01
),
"Maximum negative particle surface concentration": (1 - 0.01)
- pybamm.max(c_s_surf_n),
"Minimum positive particle surface concentration": (
pybamm.min(c_s_surf_p) - 0.01
),
"Maximum positive particle surface concentration": (1 - 0.01)
- pybamm.max(c_s_surf_p),
}
)
self.events += [
pybamm.Event("Minimum negative particle surface concentration",
pybamm.min(c_s_surf_n) - 0.01),
pybamm.Event("Maximum negative particle surface concentration",
(1 - 0.01) - pybamm.max(c_s_surf_n)),
pybamm.Event("Minimum positive particle surface concentration",
pybamm.min(c_s_surf_p) - 0.01),
pybamm.Event("Maximum positive particle surface concentration",
(1 - 0.01) - pybamm.max(c_s_surf_p)),
]
######################
# Current in the solid
######################
Expand Down Expand Up @@ -245,7 +241,8 @@ def __init__(self, name="Doyle-Fuller-Newman model"):
"right": (pybamm.Scalar(0), "Neumann"),
}
self.initial_conditions[c_e] = param.c_e_init
self.events["Zero electrolyte concentration cut-off"] = pybamm.min(c_e) - 0.002
self.events.append(pybamm.Event("Zero electrolyte concentration cut-off",
pybamm.min(c_e) - 0.002))

######################
# (Some) variables
Expand All @@ -263,8 +260,10 @@ def __init__(self, name="Doyle-Fuller-Newman model"):
"Positive electrode potential": phi_s_p,
"Terminal voltage": voltage,
}
self.events["Minimum voltage"] = voltage - param.voltage_low_cut
self.events["Maximum voltage"] = voltage - param.voltage_high_cut
self.events += [
pybamm.Event("Minimum voltage", voltage - param.voltage_low_cut),
pybamm.Event("Maximum voltage", voltage - param.voltage_high_cut),
]

@property
def default_geometry(self):
Expand Down
30 changes: 14 additions & 16 deletions pybamm/models/full_battery_models/lithium_ion/basic_spm.py
Original file line number Diff line number Diff line change
Expand Up @@ -97,20 +97,16 @@ def __init__(self, name="Single Particle Model"):
c_s_surf_n = pybamm.surf(c_s_n)
c_s_surf_p = pybamm.surf(c_s_p)
# Events specify points at which a solution should terminate
self.events.update(
{
"Minimum negative particle surface concentration": (
pybamm.min(c_s_surf_n) - 0.01
),
"Maximum negative particle surface concentration": (1 - 0.01)
- pybamm.max(c_s_surf_n),
"Minimum positive particle surface concentration": (
pybamm.min(c_s_surf_p) - 0.01
),
"Maximum positive particle surface concentration": (1 - 0.01)
- pybamm.max(c_s_surf_p),
}
)
self.events += [
pybamm.Event("Minimum negative particle surface concentration",
pybamm.min(c_s_surf_n) - 0.01),
pybamm.Event("Maximum negative particle surface concentration",
(1 - 0.01) - pybamm.max(c_s_surf_n)),
pybamm.Event("Minimum positive particle surface concentration",
pybamm.min(c_s_surf_p) - 0.01),
pybamm.Event("Maximum positive particle surface concentration",
(1 - 0.01) - pybamm.max(c_s_surf_p)),
]

# Note that the SPM does not have any algebraic equations, so the `algebraic`
# dictionary remains empty
Expand Down Expand Up @@ -164,8 +160,10 @@ def __init__(self, name="Single Particle Model"):
),
"Terminal voltage": V,
}
self.events["Minimum voltage"] = V - param.voltage_low_cut
self.events["Maximum voltage"] = V - param.voltage_high_cut
self.events += [
pybamm.Event("Minimum voltage", V - param.voltage_low_cut),
pybamm.Event("Maximum voltage", V - param.voltage_high_cut),
]

@property
def default_geometry(self):
Expand Down
24 changes: 14 additions & 10 deletions pybamm/solvers/base_solver.py
Original file line number Diff line number Diff line change
Expand Up @@ -426,7 +426,8 @@ def solve(self, model, t_eval, external_variables=None, inputs=None):

# Calculate discontinuities
discontinuities = [
event.expression.evaluate(u=inputs) for event in model.discontinuity_events_eval
event.expression.evaluate(u=inputs)
for event in model.discontinuity_events_eval
]

# make sure they are increasing in time
Expand All @@ -436,31 +437,33 @@ def solve(self, model, t_eval, external_variables=None, inputs=None):
)
# remove any identical discontinuities
discontinuities = [
v for i, v in enumerate(discontinuities)
if i==len(discontinuities)-1 or discontinuities[i] < discontinuities[i+1]
]
v for i, v in enumerate(discontinuities)
if i == len(discontinuities) - 1
or discontinuities[i] < discontinuities[i + 1]
]

# insert time points around discontinuities in t_eval
# keep track of sub sections to integrate by storing start and end indices
start_indices = [0]
end_indices = []
for dtime in discontinuities:
dindex = np.searchsorted(t_eval, dtime, side='left')
end_indices.append(dindex+1)
start_indices.append(dindex+1)
end_indices.append(dindex + 1)
start_indices.append(dindex + 1)
if t_eval[dindex] == dtime:
t_eval[dindex] += sys.float_info.epsilon
t_eval = np.insert(t_eval, dindex, dtime - sys.float_info.epsilon)
else:
t_eval = np.insert(t_eval, dindex,
[dtime - sys.float_info.epsilon, dtime + sys.float_info.epsilon])
[dtime - sys.float_info.epsilon,
dtime + sys.float_info.epsilon])
end_indices.append(len(t_eval))

old_y0 = model.y0
solution = None
for start_index, end_index in zip(start_indices, end_indices):
pybamm.logger.info("Calling solver for {} < t < {}"
.format(t_eval[start_index], t_eval[end_index-1]))
.format(t_eval[start_index], t_eval[end_index - 1]))
timer.reset()
if solution is None:
solution = self._integrate(
Expand All @@ -479,14 +482,15 @@ def solve(self, model, t_eval, external_variables=None, inputs=None):
# setup for next integration subsection
y0_guess = solution.y[:, -1]
if model.algebraic:
model.y0 = self.calculate_consistent_state(model, t_eval[end_index], y0_guess)
model.y0 = self.calculate_consistent_state(
model, t_eval[end_index], y0_guess)
else:
model.y0 = y0_guess

last_state = solution.y[:, -1]
if len(model.algebraic) > 0:
model.y0 = self.calculate_consistent_state(
model, t_eval[end_index], last_state)
model, t_eval[end_index], last_state)
else:
model.y0 = last_state

Expand Down
3 changes: 2 additions & 1 deletion pybamm/solvers/casadi_solver.py
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,8 @@ def _integrate(self, model, t_eval, inputs=None):
elif self.mode == "safe":
# Step-and-check
init_event_signs = np.sign(
np.concatenate([event(0, model.y0) for event in model.terminate_events_eval])
np.concatenate([event(0, model.y0)
for event in model.terminate_events_eval])
)
pybamm.logger.info(
"Start solving {} with {} in 'safe' mode".format(model.name, self.name)
Expand Down
1 change: 1 addition & 0 deletions tests/unit/test_solvers/test_scikits_solvers.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
import unittest
import warnings
from tests import get_mesh_for_testing, get_discretisation_for_testing
import sys


@unittest.skipIf(not pybamm.have_scikits_odes(), "scikits.odes not installed")
Expand Down

0 comments on commit d844228

Please sign in to comment.