Skip to content

Commit

Permalink
#2056 coverage
Browse files Browse the repository at this point in the history
  • Loading branch information
rtimms committed Jul 12, 2022
2 parents 8c19ab6 + 1cd386d commit 2f3d4d7
Show file tree
Hide file tree
Showing 8 changed files with 95 additions and 39 deletions.
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,15 +1,19 @@
# [Unreleased](https://github.com/pybamm-team/PyBaMM/)


## Features

- Moved general code about submodels to `BaseModel` instead of `BaseBatteryModel`, making it easier to build custom models from submodels. ([#2169](https://github.com/pybamm-team/PyBaMM/pull/2169))
- Events can now be plotted as a regular variable (under the name "Event: event_name", e.g. "Event: Minimum voltage [V]") ([#2158](https://github.com/pybamm-team/PyBaMM/pull/2158))

## Optimizations

- Added error for when solution vector gets too large, to help debug solver errors ([#2138](https://github.com/pybamm-team/PyBaMM/pull/2138))

## Bug fixes

- Fixes a bug where the SPMe always builds even when `build=False` ([#2169](https://github.com/pybamm-team/PyBaMM/pull/2169))
- Some events have been removed in the case where they are constant, i.e. can never be reached ([#2158](https://github.com/pybamm-team/PyBaMM/pull/2158))
- Raise explicit `NotImplementedError` if trying to call `bool()` on a pybamm Symbol (e.g. in an if statement condition) ([#2141](https://github.com/pybamm-team/PyBaMM/pull/2141))
- Fixed bug causing cut-off voltage to change after setting up a simulation with a model ([#2138](https://github.com/pybamm-team/PyBaMM/pull/2138))
- A single solution cycle can now be used as a starting solution for a simulation ([#2138](https://github.com/pybamm-team/PyBaMM/pull/2138))
Expand Down
14 changes: 14 additions & 0 deletions pybamm/models/base_model.py
Original file line number Diff line number Diff line change
Expand Up @@ -195,6 +195,20 @@ def variables(self, variables):
def variable_names(self):
return list(self._variables.keys())

@property
def variables_and_events(self):
"""
Returns variables and events in a single dictionary
"""
try:
return self._variables_and_events
except AttributeError:
self._variables_and_events = self.variables.copy()
self._variables_and_events.update(
{f"Event: {event.name}": event.expression for event in self.events}
)
return self._variables_and_events

@property
def events(self):
return self._events
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -88,3 +88,7 @@ def set_boundary_conditions(self, variables):
"right": (pybamm.Scalar(0), "Neumann"),
}
}

def set_events(self, variables):
# No event since the concentration is constant
pass
4 changes: 4 additions & 0 deletions pybamm/models/submodels/porosity/constant_porosity.py
Original file line number Diff line number Diff line change
Expand Up @@ -44,3 +44,7 @@ def get_fundamental_variables(self):
)

return variables

def set_events(self, variables):
# No events since porosity is constant
pass
6 changes: 4 additions & 2 deletions pybamm/solvers/solution.py
Original file line number Diff line number Diff line change
Expand Up @@ -480,12 +480,14 @@ def update(self, variables):
# ProcessedSymbolicVariable
if self.has_symbolic_inputs is True:
var = pybamm.ProcessedSymbolicVariable(
self.all_models[0].variables[key], self
self.all_models[0].variables_and_events[key], self
)

# Otherwise a standard ProcessedVariable is ok
else:
vars_pybamm = [model.variables[key] for model in self.all_models]
vars_pybamm = [
model.variables_and_events[key] for model in self.all_models
]

# Iterate through all models, some may be in the list several times and
# therefore only get set up once
Expand Down
32 changes: 29 additions & 3 deletions tests/unit/test_models/test_base_model.py
Original file line number Diff line number Diff line change
Expand Up @@ -144,6 +144,10 @@ def test_read_parameters(self):
v: {"left": (0, "Dirichlet"), "right": (h, "Neumann")},
}

# Test variables_and_events
self.assertIn("v+f+i", model.variables_and_events)
self.assertIn("Event: u=e", model.variables_and_events)

self.assertEqual(
set([x.name for x in model.parameters]),
set([x.name for x in [a, b, c, d, e, f, g, h, i]]),
Expand Down Expand Up @@ -955,8 +959,8 @@ def test_set_variables_error(self):
with self.assertRaisesRegex(ValueError, "not var"):
model.variables = {"not var": var}

def test_build(self):
class Submodel(pybamm.BaseSubModel):
def test_build_submodels(self):
class Submodel1(pybamm.BaseSubModel):
def __init__(self, param, domain, options=None):
super().__init__(param, domain, options=options)

Expand All @@ -981,8 +985,30 @@ def set_initial_conditions(self, variables):
v = variables["v"]
self.initial_conditions = {u: 0, v: 0}

def set_events(self, variables):
u = variables["u"]
self.events.append(
pybamm.Event(
"Large u",
u - 200,
pybamm.EventType.TERMINATION,
)
)

class Submodel2(pybamm.BaseSubModel):
def __init__(self, param, domain, options=None):
super().__init__(param, domain, options=options)

def get_coupled_variables(self, variables):
u = variables["u"]
variables.update({"w": 2 * u})
return variables

model = pybamm.BaseModel()
model.submodels = {"submodel": Submodel(None, "Negative")}
model.submodels = {
"submodel 1": Submodel1(None, "Negative"),
"submodel 2": Submodel2(None, "Negative"),
}
self.assertFalse(model._built)
model.build_model()
self.assertTrue(model._built)
Expand Down
12 changes: 5 additions & 7 deletions tests/unit/test_plotting/test_quick_plot.py
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,11 @@ def test_simple_ode_model(self):
"c broadcasted positive electrode": pybamm.PrimaryBroadcast(
c, "positive particle"
),
"Variable with a very long name": a,
"2D variable": pybamm.FullBroadcast(
1, "negative particle", {"secondary": "negative electrode"}
),
"NaN variable": pybamm.Scalar(np.nan),
}
model._timescale = pybamm.Scalar(1)

Expand Down Expand Up @@ -116,7 +121,6 @@ def test_simple_ode_model(self):
quick_plot.slider_update(0.01)

# Test longer name
model.variables["Variable with a very long name"] = model.variables["a"]
quick_plot = pybamm.QuickPlot(solution, ["Variable with a very long name"])
quick_plot.plot(0)

Expand Down Expand Up @@ -187,11 +191,6 @@ def test_simple_ode_model(self):
pybamm.QuickPlot(solution, ["a"], spatial_unit="bad unit")

# Test 2D variables
model.variables["2D variable"] = disc.process_symbol(
pybamm.FullBroadcast(
1, "negative particle", {"secondary": "negative electrode"}
)
)
quick_plot = pybamm.QuickPlot(solution, ["2D variable"])
quick_plot.plot(0)
quick_plot.dynamic_plot(testing=True)
Expand Down Expand Up @@ -256,7 +255,6 @@ def test_simple_ode_model(self):
)

# No variable can be NaN
model.variables["NaN variable"] = disc.process_symbol(pybamm.Scalar(np.nan))
with self.assertRaisesRegex(
ValueError, "All-NaN variable 'NaN variable' provided"
):
Expand Down
58 changes: 31 additions & 27 deletions tests/unit/test_solvers/test_scipy_solver.py
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,9 @@ def test_model_solver_with_event_python(self):
np.testing.assert_equal(solution.t_event[0], solution.t[-1])
np.testing.assert_array_equal(solution.y_event[:, 0], solution.y[:, -1])

# Test event in solution variables_and_events
np.testing.assert_array_almost_equal(solution["Event: var=0.5"].data[-1], 0)

def test_model_solver_ode_with_jacobian_python(self):
# Create model
model = pybamm.BaseModel()
Expand Down Expand Up @@ -513,12 +516,11 @@ def test_solve_sensitivity_scalar_var_scalar_input(self):

# Solve
# Make sure that passing in extra options works
solver = pybamm.ScipySolver(
rtol=1e-10, atol=1e-10
)
solver = pybamm.ScipySolver(rtol=1e-10, atol=1e-10)
t_eval = np.linspace(0, 1, 80)
solution = solver.solve(model, t_eval, inputs={"p": 0.1},
calculate_sensitivities=True)
solution = solver.solve(
model, t_eval, inputs={"p": 0.1}, calculate_sensitivities=True
)
np.testing.assert_array_equal(solution.t, t_eval)
np.testing.assert_allclose(solution.y[0], np.exp(0.1 * solution.t))
np.testing.assert_allclose(
Expand Down Expand Up @@ -549,13 +551,13 @@ def test_solve_sensitivity_scalar_var_scalar_input(self):

# Solve
# Make sure that passing in extra options works
solver = pybamm.ScipySolver(
rtol=1e-10, atol=1e-10
)
solver = pybamm.ScipySolver(rtol=1e-10, atol=1e-10)
t_eval = np.linspace(0, 1, 80)
solution = solver.solve(
model, t_eval, inputs={"p": 0.1, "q": 2, "r": -1, "s": 0.5},
calculate_sensitivities=True
model,
t_eval,
inputs={"p": 0.1, "q": 2, "r": -1, "s": 0.5},
calculate_sensitivities=True,
)
np.testing.assert_allclose(solution.y[0], -1 + 0.2 * solution.t)
np.testing.assert_allclose(
Expand Down Expand Up @@ -625,8 +627,9 @@ def test_solve_sensitivity_vector_var_scalar_input(self):
# Solve - scalar input
solver = pybamm.ScipySolver()
t_eval = np.linspace(0, 1)
solution = solver.solve(model, t_eval, inputs={"param": 7},
calculate_sensitivities=True)
solution = solver.solve(
model, t_eval, inputs={"param": 7}, calculate_sensitivities=True
)
np.testing.assert_array_almost_equal(
solution["var"].data,
np.tile(2 * np.exp(-7 * t_eval), (n, 1)),
Expand Down Expand Up @@ -657,13 +660,13 @@ def test_solve_sensitivity_vector_var_scalar_input(self):

# Solve
# Make sure that passing in extra options works
solver = pybamm.ScipySolver(
rtol=1e-10, atol=1e-10
)
solver = pybamm.ScipySolver(rtol=1e-10, atol=1e-10)
t_eval = np.linspace(0, 1, 80)
solution = solver.solve(
model, t_eval, inputs={"p": 0.1, "q": 2, "r": -1, "s": 0.5},
calculate_sensitivities=True
model,
t_eval,
inputs={"p": 0.1, "q": 2, "r": -1, "s": 0.5},
calculate_sensitivities=True,
)
np.testing.assert_allclose(solution.y, np.tile(-1 + 0.2 * solution.t, (n, 1)))
np.testing.assert_allclose(
Expand Down Expand Up @@ -737,12 +740,14 @@ def test_solve_sensitivity_vector_var_vector_input(self):
n = disc.mesh["negative electrode"].npts

# Solve - constant input
solver = pybamm.ScipySolver(
rtol=1e-10, atol=1e-10
)
solver = pybamm.ScipySolver(rtol=1e-10, atol=1e-10)
t_eval = np.linspace(0, 1)
solution = solver.solve(model, t_eval, inputs={"param": 7 * np.ones(n)},
calculate_sensitivities=True)
solution = solver.solve(
model,
t_eval,
inputs={"param": 7 * np.ones(n)},
calculate_sensitivities=True,
)
l_n = mesh["negative electrode"].edges[-1]
np.testing.assert_array_almost_equal(
solution["var"].data,
Expand All @@ -765,13 +770,12 @@ def test_solve_sensitivity_vector_var_vector_input(self):
)

# Solve - linspace input
solver = pybamm.ScipySolver(
rtol=1e-10, atol=1e-10
)
solver = pybamm.ScipySolver(rtol=1e-10, atol=1e-10)
t_eval = np.linspace(0, 1)
p_eval = np.linspace(1, 2, n)
solution = solver.solve(model, t_eval, inputs={"param": p_eval},
calculate_sensitivities=True)
solution = solver.solve(
model, t_eval, inputs={"param": p_eval}, calculate_sensitivities=True
)
l_n = mesh["negative electrode"].edges[-1]
np.testing.assert_array_almost_equal(
solution["var"].data, 2 * np.exp(-p_eval[:, np.newaxis] * t_eval), decimal=4
Expand Down

0 comments on commit 2f3d4d7

Please sign in to comment.