Merge pull request #2213 from pybamm-team/callback-bug

fix bug in callbacks

CHANGELOG.md

@@ -2,7 +2,6 @@
 
 # [v22.7](https://github.com/pybamm-team/PyBaMM/tree/v22.7) - 2022-07-31
 
-
 ## 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))
@@ -21,6 +20,7 @@
 
 ## Bug fixes
 
+-   Fixed error reporting for simulation with experiment ([#2213](https://github.com/pybamm-team/PyBaMM/pull/2213))
 -   Fixed a bug in `Simulation` that caused initial conditions to change when solving an experiment multiple times ([#2204](https://github.com/pybamm-team/PyBaMM/pull/2204))
 -   Fixed labels and ylims in `plot_voltage_components`([#2183](https://github.com/pybamm-team/PyBaMM/pull/2183))
 -   Fixed 2D interpolant ([#2180](https://github.com/pybamm-team/PyBaMM/pull/2180))

pybamm/callbacks.py

@@ -222,22 +222,16 @@ def on_experiment_end(self, logs):
         self.logger.notice("Finish experiment simulation, took {}".format(elapsed_time))
 
     def on_experiment_error(self, logs):
-        pass
+        error = logs["error"]
+        pybamm.logger.error(f"Simulation error: {error}")
 
     def on_experiment_infeasible(self, logs):
         termination = logs["termination"]
         cycle_num = logs["cycle number"][0]
         step_num = logs["step number"][0]
         operating_conditions = logs["step operating conditions"]
-        if step_num == 1:
-            cycle_num -= 1
-            up_to_step = ""
-        else:
-            up_to_step = f", up to step {step_num-1}"
         self.logger.warning(
             f"\n\n\tExperiment is infeasible: '{termination}' was "
             f"triggered during '{operating_conditions}'. The returned solution only "
-            f"contains the first {cycle_num} cycles{up_to_step}. "
-            "Try reducing the current, shortening the time interval, or reducing the "
-            "period.\n\n"
+            f"contains up to step {step_num} of cycle {cycle_num}. "
         )

pybamm/simulation.py

@@ -858,6 +858,7 @@ def solve(
                         or step_solution.termination == "final time"
                         or "[experiment]" in step_solution.termination
                     ):
+                        callbacks.on_experiment_infeasible(logs)
                         feasible = False
                         break
 
@@ -897,6 +898,7 @@ def solve(
                         capacity_stop = None
                     logs["stopping conditions"]["capacity"] = capacity_stop
 
+                logs["elapsed time"] = timer.time()
                 callbacks.on_cycle_end(logs)
 
                 # Break if stopping conditions are met
@@ -913,7 +915,6 @@ def solve(
 
                 # Break if the experiment is infeasible (or errored)
                 if feasible is False:
-                    callbacks.on_experiment_infeasible(logs)
                     break
 
             if self.solution is not None and len(all_cycle_solutions) > 0:

pybamm/solvers/casadi_solver.py

@@ -261,13 +261,13 @@ def _integrate(self, model, t_eval, inputs_dict=None):
                         dt_max = dt
                     count += 1
                     if count >= self.max_step_decrease_count:
+                        t_dim = t * model.timescale_eval
+                        dt_max_dim = dt_max * model.timescale_eval
                         raise pybamm.SolverError(
-                            "Maximum number of decreased steps occurred at t={}. Try "
-                            "solving the model up to this time only or reducing dt_max "
-                            "(currently, dt_max={})."
-                            "".format(
-                                t * model.timescale_eval, dt_max * model.timescale_eval
-                            )
+                            f"Maximum number of decreased steps occurred at t={t_dim}. "
+                            "Try solving the model up to this time only or reducing "
+                            f"dt_max (currently, dt_max={dt_max_dim}) and/or reducing "
+                            "the size of the time steps or period of the expeeriment."
                         )
                 # Check if the sign of an event changes, if so find an accurate
                 # termination point and exit
@@ -547,20 +547,11 @@ def create_integrator(self, model, inputs, t_eval=None, use_event_switch=False):
             rhs = model.casadi_rhs
             algebraic = model.casadi_algebraic
 
-            # When not in DEBUG mode (level=10), suppress warnings from CasADi
-            if (
-                pybamm.logger.getEffectiveLevel() == 10
-                or pybamm.settings.debug_mode is True
-            ):
-                show_eval_warnings = True
-            else:
-                show_eval_warnings = False
-
             options = {
+                "show_eval_warnings": False,
                 **self.extra_options_setup,
                 "reltol": self.rtol,
                 "abstol": self.atol,
-                "show_eval_warnings": show_eval_warnings,
             }
 
             # set up and solve

tests/unit/test_experiments/test_simulation_with_experiment.py

@@ -449,7 +449,7 @@ def test_run_experiment_half_cell(self):
 
     def test_run_experiment_lead_acid(self):
         experiment = pybamm.Experiment(
-            [("Discharge at C/20 until 1.9V", "Charge at 1C until 2.1 V")]
+            [("Discharge at C/20 until 10.5V", "Charge at C/20 until 12.5 V")]
         )
         model = pybamm.lead_acid.Full()
         sim = pybamm.Simulation(model, experiment=experiment)