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

#738 check model option #739

Merged
merged 4 commits into from
Nov 26, 2019
Merged
Changes from all commits
Commits
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
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -21,6 +21,7 @@

## Optimizations

- Added an option to skip model checks during discretisation, which could be slow for large models ([#739](https://github.com/pybamm-team/PyBaMM/pull/739))
- Use CasADi's automatic differentation algorithms by default when solving a model ([#714](https://github.com/pybamm-team/PyBaMM/pull/714))
- Avoid re-checking size when making a copy of an `Index` object ([#656](https://github.com/pybamm-team/PyBaMM/pull/656))
- Avoid recalculating `_evaluation_array` when making a copy of a `StateVector` object ([#653](https://github.com/pybamm-team/PyBaMM/pull/653))
Original file line number Diff line number Diff line change
@@ -33,4 +33,4 @@ Negative current collector thermal conductivity [W.m-1.K-1],401,,
Positive current collector thermal conductivity [W.m-1.K-1],237,,
,,,
# Electrical,,,
Cell capacity [A.h],0.68,,24 Ah/m2 * 0.137m * 0.207m
Cell capacity [A.h],0.680616,,24 Ah/m2 * 0.137m * 0.207m
14 changes: 11 additions & 3 deletions pybamm/discretisations/discretisation.py
Original file line number Diff line number Diff line change
@@ -82,7 +82,7 @@ def bcs(self, value):
# reset discretised_symbols
self._discretised_symbols = {}

def process_model(self, model, inplace=True):
def process_model(self, model, inplace=True, check_model=True):
"""Discretise a model.
Currently inplace, could be changed to return a new model.

@@ -91,9 +91,15 @@ def process_model(self, model, inplace=True):
model : :class:`pybamm.BaseModel`
Model to dicretise. Must have attributes rhs, initial_conditions and
boundary_conditions (all dicts of {variable: equation})
inplace: bool, optional
inplace : bool, optional
If True, discretise the model in place. Otherwise, return a new
discretised model. Default is True.
check_model : bool, optional
If True, model checks are performed after discretisation. For large
systems these checks can be slow, so can be skipped by setting this
option to False. When developing, testing or debugging it is recommened
to leave this option as True as it may help to identify any errors.
Default is True.

Returns
-------
@@ -173,7 +179,9 @@ def process_model(self, model, inplace=True):
model_disc.mass_matrix = self.create_mass_matrix(model_disc)

# Check that resulting model makes sense
self.check_model(model_disc)
if check_model:
pybamm.logger.info("Performing model checks for {}".format(model.name))
self.check_model(model_disc)

pybamm.logger.info("Finish discretising {}".format(model.name))

19 changes: 15 additions & 4 deletions pybamm/simulation.py
Original file line number Diff line number Diff line change
@@ -98,13 +98,19 @@ def set_parameters(self):
)
self._parameter_values.process_geometry(self._geometry)

def build(self):
def build(self, check_model=True):
"""
A method to build the model into a system of matrices and vectors suitable for
performing numerical computations. If the model has already been built or
solved then this function will have no effect. If you want to rebuild,
first use "reset()". This method will automatically set the parameters
if they have not already been set.

Parameters
----------
check_model : bool, optional
If True, model checks are performed after discretisation (see
:meth:`pybamm.Discretisation.process_model`). Default is True.
"""

if self.built_model:
@@ -113,9 +119,11 @@ def build(self):
self.set_parameters()
self._mesh = pybamm.Mesh(self._geometry, self._submesh_types, self._var_pts)
self._disc = pybamm.Discretisation(self._mesh, self._spatial_methods)
self._built_model = self._disc.process_model(self._model, inplace=False)
self._built_model = self._disc.process_model(
self._model, inplace=False, check_model=check_model
)

def solve(self, t_eval=None, solver=None):
def solve(self, t_eval=None, solver=None, check_model=True):
"""
A method to solve the model. This method will automatically build
and set the model parameters if not already done so.
@@ -129,8 +137,11 @@ def solve(self, t_eval=None, solver=None):
non-dimensional time of 1.
solver : :class:`pybamm.BaseSolver`
The solver to use to solve the model.
check_model : bool, optional
If True, model checks are performed after discretisation (see
:meth:`pybamm.Discretisation.process_model`). Default is True.
"""
self.build()
self.build(check_model=check_model)

if t_eval is None:
try:
17 changes: 17 additions & 0 deletions tests/unit/test_discretisations/test_discretisation.py
Original file line number Diff line number Diff line change
@@ -852,6 +852,23 @@ def test_check_tab_bcs_error(self):
with self.assertRaisesRegex(pybamm.ModelError, "Boundary conditions"):
disc.check_tab_conditions(b, bcs)

def test_process_with_no_check(self):
# create model
whole_cell = ["negative electrode", "separator", "positive electrode"]
c = pybamm.Variable("c", domain=whole_cell)
N = pybamm.grad(c)
model = pybamm.BaseModel()
model.rhs = {c: pybamm.div(N)}
model.initial_conditions = {c: pybamm.Scalar(3)}
model.boundary_conditions = {
c: {"left": (0, "Neumann"), "right": (0, "Neumann")}
}
model.variables = {"c": c, "N": N}

# create discretisation
disc = get_discretisation_for_testing()
disc.process_model(model, check_model=False)


if __name__ == "__main__":
print("Add -v for more debug output")
4 changes: 1 addition & 3 deletions tests/unit/test_parameters/test_update_parameters.py
Original file line number Diff line number Diff line change
@@ -47,7 +47,7 @@ def test_update_model(self):
model2 = pybamm.lithium_ion.SPM()
modeltest2 = tests.StandardModelTest(model2)
modeltest2.test_all(skip_output_tests=True)
self.assertEqual(model2.variables["Current [A]"].evaluate(), 0.68)
self.assertEqual(model2.variables["Current [A]"].evaluate(), 0.680616)
# process and solve with updated parameter values
parameter_values_update = pybamm.ParameterValues(
chemistry=pybamm.parameter_sets.Marquis2019
@@ -73,8 +73,6 @@ def test_update_model(self):
modeltest3.test_solving(t_eval=t_eval)
Y3 = modeltest3.solution.y

# function.parameters should be pybamm.Scalar(0), but parameters_eval s
# should be a float
self.assertIsInstance(model3.variables["Current [A]"], pybamm.Scalar)
self.assertEqual(model3.variables["Current [A]"].evaluate(), 0.0)

7 changes: 7 additions & 0 deletions tests/unit/test_simulation.py
Original file line number Diff line number Diff line change
@@ -71,6 +71,13 @@ def test_solve(self):

self.assertEqual(sim._solution, None)

# test solve without check
sim.reset()
sim.solve(check_model=False)
for val in list(sim.built_model.rhs.values()):
self.assertFalse(val.has_symbol_of_classes(pybamm.Parameter))
self.assertTrue(val.has_symbol_of_classes(pybamm.Matrix))

def test_reuse_commands(self):

sim = pybamm.Simulation(pybamm.lithium_ion.SPM())