From 4b3df28f04ee9efaf2339a345afe7de9782798cd Mon Sep 17 00:00:00 2001 From: Valentin Sulzer Date: Tue, 18 Feb 2020 09:45:42 -0500 Subject: [PATCH 1/7] #835 add hacky solution --- pybamm/discretisations/discretisation.py | 12 +++++------- pybamm/quick_plot.py | 4 +++- pybamm/solvers/base_solver.py | 10 ++++------ 3 files changed, 12 insertions(+), 14 deletions(-) diff --git a/pybamm/discretisations/discretisation.py b/pybamm/discretisations/discretisation.py index 790098fd58..9496fadf2b 100644 --- a/pybamm/discretisations/discretisation.py +++ b/pybamm/discretisations/discretisation.py @@ -178,9 +178,7 @@ 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 @@ -192,9 +190,9 @@ def process_model(self, model, inplace=True, check_model=True): ) # Check that resulting model makes sense - if check_model: - pybamm.logger.info("Performing model checks for {}".format(model.name)) - 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)) @@ -402,7 +400,7 @@ def process_initial_conditions(self, model): # check that all initial conditions are set processed_concatenated_initial_conditions = self._concatenate_in_order( processed_initial_conditions, check_complete=True - ).evaluate(0, None) + ) return processed_initial_conditions, processed_concatenated_initial_conditions diff --git a/pybamm/quick_plot.py b/pybamm/quick_plot.py index e320adffd3..4e5a54b8fd 100644 --- a/pybamm/quick_plot.py +++ b/pybamm/quick_plot.py @@ -110,7 +110,9 @@ def __init__( variables["r_p [m]"] / variables["r_p"] ).evaluate()[-1] if "Time [h]" and "Time" in variables: - self.time_scale = (variables["Time [h]"] / variables["Time"]).evaluate(t=1) + self.time_scale = (variables["Time [h]"] / variables["Time"]).evaluate( + t=1, u={k: v[0] for k, v in solutions[0].inputs.items()} + ) # Time parameters self.ts = [solution.t for solution in solutions] diff --git a/pybamm/solvers/base_solver.py b/pybamm/solvers/base_solver.py index 7b11c219e9..d90e6b4a61 100644 --- a/pybamm/solvers/base_solver.py +++ b/pybamm/solvers/base_solver.py @@ -115,7 +115,7 @@ def set_up(self, model, inputs=None): """ inputs = inputs or {} - y0 = model.concatenated_initial_conditions + y0 = model.concatenated_initial_conditions.evaluate(0, None, inputs) # Check model.algebraic for ode solvers if self.ode_solver is True and len(model.algebraic) > 0: @@ -135,9 +135,7 @@ def set_up(self, model, inputs=None): if model.convert_to_format != "casadi": simp = pybamm.Simplification() # Create Jacobian from concatenated rhs and algebraic - y = pybamm.StateVector( - slice(0, np.size(model.concatenated_initial_conditions)) - ) + y = pybamm.StateVector(slice(0, np.size(y0))) # set up Jacobian object, for re-use of dict jacobian = pybamm.Jacobian() else: @@ -267,13 +265,13 @@ def report(string): residuals, residuals_eval, jacobian_eval = process(all_states, "residuals") model.residuals_eval = residuals_eval model.jacobian_eval = jacobian_eval - y0_guess = model.concatenated_initial_conditions.flatten() + y0_guess = y0.flatten() model.y0 = self.calculate_consistent_state(model, 0, y0_guess) else: # can use DAE solver to solve ODE model model.residuals_eval = Residuals(rhs, "residuals", model) model.jacobian_eval = jac_rhs - model.y0 = model.concatenated_initial_conditions[:, 0] + model.y0 = y0.flatten() # Save CasADi functions for the CasADi solver # Note: when we pass to casadi the ode part of the problem must be in explicit From 8f15edbd15951a9e3f43713141624f6330ce466b Mon Sep 17 00:00:00 2001 From: Valentin Sulzer Date: Wed, 19 Feb 2020 08:43:46 -0500 Subject: [PATCH 2/7] #835 fix scikit test --- tests/unit/test_solvers/test_scikits_solvers.py | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/unit/test_solvers/test_scikits_solvers.py b/tests/unit/test_solvers/test_scikits_solvers.py index 7e6974c57e..756f6465d8 100644 --- a/tests/unit/test_solvers/test_scikits_solvers.py +++ b/tests/unit/test_solvers/test_scikits_solvers.py @@ -596,6 +596,7 @@ def test_ode_solver_fail_with_dae(self): model = pybamm.BaseModel() a = pybamm.Scalar(1) model.algebraic = {a: a} + model.concatenated_initial_conditions = a solver = pybamm.ScikitsOdeSolver() with self.assertRaisesRegex(pybamm.SolverError, "Cannot use ODE solver"): solver.set_up(model) From a91da418018c0c1f66ab2db3d029e688fe1bdc2c Mon Sep 17 00:00:00 2001 From: Valentin Sulzer Date: Wed, 19 Feb 2020 09:02:54 -0500 Subject: [PATCH 3/7] #835 add changelog and fix tom's tests --- CHANGELOG.md | 3 +- pybamm/discretisations/discretisation.py | 4 +- pybamm/solvers/base_solver.py | 4 +- .../test_solvers/test_external_variables.py | 65 +++++++++++++++++++ 4 files changed, 71 insertions(+), 5 deletions(-) create mode 100644 tests/integration/test_solvers/test_external_variables.py diff --git a/CHANGELOG.md b/CHANGELOG.md index 5fca98a703..3e6cf962d2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -53,7 +53,7 @@ ## Bug fixes -- Time for solver should now be given in seconds ([#832](https://github.com/pybamm-team/PyBaMM/pull/832)) +- Moved evaluation of initial conditions to solver ([#839](https://github.com/pybamm-team/PyBaMM/pull/839)) - Fixed a bug where the first line of the data wasn't loaded when parameters are loaded from data ([#819](https://github.com/pybamm-team/PyBaMM/pull/819)) - Made `graphviz` an optional dependency ([#810](https://github.com/pybamm-team/PyBaMM/pull/810)) - Fixed examples to run with basic pip installation ([#800](https://github.com/pybamm-team/PyBaMM/pull/800)) @@ -74,6 +74,7 @@ ## Breaking changes +- Time for solver should now be given in seconds ([#832](https://github.com/pybamm-team/PyBaMM/pull/832)) - Model events are now represented as a list of `pybamm.Event` ([#759](https://github.com/pybamm-team/PyBaMM/issues/759) - Removed `ParameterValues.update_model`, whose functionality is now replaced by `InputParameter` ([#801](https://github.com/pybamm-team/PyBaMM/pull/801)) - Removed `Outer` and `Kron` nodes as no longer used ([#777](https://github.com/pybamm-team/PyBaMM/pull/777)) diff --git a/pybamm/discretisations/discretisation.py b/pybamm/discretisations/discretisation.py index c5883583f9..48681b3f1d 100644 --- a/pybamm/discretisations/discretisation.py +++ b/pybamm/discretisations/discretisation.py @@ -994,12 +994,12 @@ def check_initial_conditions(self, model): """Check initial conditions are a numpy array""" # Individual for var, eqn in model.initial_conditions.items(): - assert type(eqn.evaluate(0, None)) is np.ndarray, pybamm.ModelError( + assert type(eqn.evaluate(t=0, u="shape test")) is np.ndarray, pybamm.ModelError( """ initial_conditions must be numpy array after discretisation but they are {} for variable '{}'. """.format( - type(eqn.evaluate(0, None)), var + type(eqn.evaluate(t=0, u="shape test")), var ) ) # Concatenated diff --git a/pybamm/solvers/base_solver.py b/pybamm/solvers/base_solver.py index 44ef300ee3..a137458d02 100644 --- a/pybamm/solvers/base_solver.py +++ b/pybamm/solvers/base_solver.py @@ -560,7 +560,7 @@ def solve(self, model, t_eval, external_variables=None, inputs=None): # Add model and inputs to solution solution.model = model - solution.inputs = inputs + solution.inputs = ext_and_inputs # Identify the event that caused termination termination = self.get_termination_reason(solution, model.events) @@ -658,7 +658,7 @@ def step( # Add model and inputs to solution solution.model = model - solution.inputs = inputs + solution.inputs = ext_and_inputs # Identify the event that caused termination termination = self.get_termination_reason(solution, model.events) diff --git a/tests/integration/test_solvers/test_external_variables.py b/tests/integration/test_solvers/test_external_variables.py new file mode 100644 index 0000000000..cd2a43abb9 --- /dev/null +++ b/tests/integration/test_solvers/test_external_variables.py @@ -0,0 +1,65 @@ +# +# Test solvers with external variables +# +import pybamm +import numpy as np +import unittest + + +class TestExternalVariables(unittest.TestCase): + def test_on_dfn(self): + e_height = 0.25 + + model = pybamm.lithium_ion.DFN() + geometry = model.default_geometry + param = model.default_parameter_values + param.update({"Electrode height [m]": "[input]"}) + param.process_model(model) + param.process_geometry(geometry) + inputs = {"Electrode height [m]": e_height} + var = pybamm.standard_spatial_vars + var_pts = {var.x_n: 5, var.x_s: 5, var.x_p: 5, var.r_n: 10, var.r_p: 10} + spatial_methods = model.default_spatial_methods + + solver = pybamm.CasadiSolver() + sim = pybamm.Simulation( + model=model, + geometry=geometry, + parameter_values=param, + var_pts=var_pts, + spatial_methods=spatial_methods, + solver=solver, + ) + sim.solve(t_eval=np.linspace(0, 3600, 100), inputs=inputs) + + def test_external_variables_SPMe(self): + model_options = { + "thermal": "x-lumped", + "external submodels": ["thermal"], + } + model = pybamm.lithium_ion.SPMe(model_options) + sim = pybamm.Simulation(model) + t_eval = np.linspace(0, 100, 3) + T_av = 0 + for i in np.arange(1, len(t_eval) - 1): + dt = t_eval[i + 1] - t_eval[i] + external_variables = {"X-averaged cell temperature": T_av} + T_av += 1 + sim.step(dt, external_variables=external_variables) + var = "Terminal voltage [V]" + t = sim.solution.t[-1] + y = sim.solution.y[:, -1] + u = external_variables + sim.built_model.variables[var].evaluate(t, y, u) + sim.solution[var](t) + + +if __name__ == "__main__": + import sys + + print("Add -v for more debug output") + + if "-v" in sys.argv: + debug = True + pybamm.settings.debug_mode = True + unittest.main() From b2f793b1de122e28c48b1f46cb1cdfa056916481 Mon Sep 17 00:00:00 2001 From: Valentin Sulzer Date: Wed, 19 Feb 2020 09:16:53 -0500 Subject: [PATCH 4/7] #835 flake8 --- pybamm/discretisations/discretisation.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/pybamm/discretisations/discretisation.py b/pybamm/discretisations/discretisation.py index 48681b3f1d..abace025f3 100644 --- a/pybamm/discretisations/discretisation.py +++ b/pybamm/discretisations/discretisation.py @@ -994,7 +994,9 @@ def check_initial_conditions(self, model): """Check initial conditions are a numpy array""" # Individual for var, eqn in model.initial_conditions.items(): - assert type(eqn.evaluate(t=0, u="shape test")) is np.ndarray, pybamm.ModelError( + assert ( + type(eqn.evaluate(t=0, u="shape test")) is np.ndarray + ), pybamm.ModelError( """ initial_conditions must be numpy array after discretisation but they are {} for variable '{}'. From 099915a7bfacab3d7acf2abe3a83dc50ef8fd0d9 Mon Sep 17 00:00:00 2001 From: Valentin Sulzer Date: Wed, 19 Feb 2020 10:16:17 -0500 Subject: [PATCH 5/7] #835 change initialization of steps to use old solution --- pybamm/solvers/base_solver.py | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/pybamm/solvers/base_solver.py b/pybamm/solvers/base_solver.py index a137458d02..536734fabe 100644 --- a/pybamm/solvers/base_solver.py +++ b/pybamm/solvers/base_solver.py @@ -48,7 +48,6 @@ def __init__( self.max_steps = max_steps self.models_set_up = set() - self.model_step_times = {} # Defaults, can be overwritten by specific solver self.name = "Base solver" @@ -630,20 +629,22 @@ def step( ext_and_inputs = {**external_variables, **inputs} # Run set up on first step - if model not in self.model_step_times: + if old_solution is None: pybamm.logger.info( "Start stepping {} with {}".format(model.name, self.name) ) self.set_up(model, ext_and_inputs) - self.model_step_times[model] = 0.0 + t = 0.0 set_up_time = timer.time() else: + # initialize with old solution + t = old_solution.t[-1] + model.y0 = old_solution.y[:, -1] set_up_time = 0 # Non-dimensionalise dt dt_dimensionless = dt / model.timescale_eval # Step - t = self.model_step_times[model] t_eval = np.linspace(t, t + dt_dimensionless, npts) # Set inputs and external self.set_inputs(model, ext_and_inputs) @@ -663,10 +664,6 @@ def step( # Identify the event that caused termination termination = self.get_termination_reason(solution, model.events) - # Set self.t and self.y0 to their values at the final step - self.model_step_times[model] = solution.t[-1] - model.y0 = solution.y[:, -1] - pybamm.logger.debug("Finish stepping {} ({})".format(model.name, termination)) if set_up_time: pybamm.logger.debug( From f5b0ae7478fd2327a7a35f9b351823c6c065deae Mon Sep 17 00:00:00 2001 From: Valentin Sulzer Date: Wed, 19 Feb 2020 11:46:24 -0500 Subject: [PATCH 6/7] #835 move 'save' functionality to the solver --- pybamm/simulation.py | 28 +++++++++------------------- pybamm/solvers/base_solver.py | 14 +++++++++++--- 2 files changed, 20 insertions(+), 22 deletions(-) diff --git a/pybamm/simulation.py b/pybamm/simulation.py index 75d7f14274..6e1be1c6b2 100644 --- a/pybamm/simulation.py +++ b/pybamm/simulation.py @@ -471,25 +471,15 @@ def step( if solver is None: solver = self.solver - if save is False: - # Don't pass previous solution - self._solution = solver.step( - None, - self.built_model, - dt, - npts=npts, - external_variables=external_variables, - inputs=inputs, - ) - else: - self._solution = solver.step( - self._solution, - self.built_model, - dt, - npts=npts, - external_variables=external_variables, - inputs=inputs, - ) + self._solution = solver.step( + self._solution, + self.built_model, + dt, + npts=npts, + external_variables=external_variables, + inputs=inputs, + save=save, + ) def get_variable_array(self, *variables): """ diff --git a/pybamm/solvers/base_solver.py b/pybamm/solvers/base_solver.py index 536734fabe..8b9fd509dd 100644 --- a/pybamm/solvers/base_solver.py +++ b/pybamm/solvers/base_solver.py @@ -575,7 +575,14 @@ def solve(self, model, t_eval, external_variables=None, inputs=None): return solution def step( - self, old_solution, model, dt, npts=2, external_variables=None, inputs=None + self, + old_solution, + model, + dt, + npts=2, + external_variables=None, + inputs=None, + save=True, ): """ Step the solution of the model forward by a given time increment. The @@ -599,7 +606,8 @@ def step( values at the current time inputs : dict, optional Any input parameters to pass to the model when solving - + save : bool + Turn on to store the solution of all previous timesteps Raises ------ @@ -677,7 +685,7 @@ def step( pybamm.logger.debug( "Step time: {}".format(timer.format(solution.solve_time)) ) - if old_solution is None: + if save is False or old_solution is None: return solution else: return old_solution + solution From 0a5ce0bee14feb84cc6fbfe65edcefeee0525312 Mon Sep 17 00:00:00 2001 From: Valentin Sulzer Date: Wed, 19 Feb 2020 13:34:00 -0500 Subject: [PATCH 7/7] #835 codacy --- pybamm/discretisations/discretisation.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pybamm/discretisations/discretisation.py b/pybamm/discretisations/discretisation.py index abace025f3..b74ac08a72 100644 --- a/pybamm/discretisations/discretisation.py +++ b/pybamm/discretisations/discretisation.py @@ -994,8 +994,8 @@ def check_initial_conditions(self, model): """Check initial conditions are a numpy array""" # Individual for var, eqn in model.initial_conditions.items(): - assert ( - type(eqn.evaluate(t=0, u="shape test")) is np.ndarray + assert isinstance( + eqn.evaluate(t=0, u="shape test"), np.ndarray ), pybamm.ModelError( """ initial_conditions must be numpy array after discretisation but they are