pybamm-team / PyBaMM

Fast and flexible physics-based battery models in Python
https://www.pybamm.org/
BSD 3-Clause "New" or "Revised" License
1.06k stars 528 forks source link

Simulation of 2D pouch cell model fails when experiment has more than one step #1549

Open DavidMStraub opened 3 years ago

DavidMStraub commented 3 years ago

Describe the bug

Simulation of 2D pouch cell model fails when experiment has more than one step.

This is almost the same code as in #1547, but I believe it's a different error.

To Reproduce

import pybamm
options = {"cell geometry": "pouch", "thermal": "x-lumped", "dimensionality": 2}
model = pybamm.lithium_ion.SPMe(options=options)
experiment = pybamm.Experiment([
    "Discharge with 5 C for 1 minute",
    "Charge with 1 C for 10 seconds",
])
sim = pybamm.Simulation(model, experiment=experiment)
sim.solve(calc_esoh=False)

It works when removing the second step in the experiment (see #1547).

Expected behaviour

Solve

Screenshots

---------------------------------------------------------------------------
NotImplementedError                       Traceback (most recent call last)
<timed exec> in <module>

~/Code/PyBaMM/pybamm/simulation.py in solve(self, t_eval, solver, check_model, save_at_cycles, calc_esoh, starting_solution, initial_soc, **kwargs)
    784                     # Make sure we take at least 2 timesteps
    785                     npts = max(int(round(dt / exp_inputs["period"])) + 1, 2)
--> 786                     step_solution = solver.step(
    787                         current_solution,
    788                         model,

~/Code/PyBaMM/pybamm/solvers/base_solver.py in step(self, old_solution, model, dt, npts, external_variables, inputs, save)
    991             else:
    992                 model.y0 = (
--> 993                     model.set_initial_conditions_from(old_solution, inplace=False)
    994                     .concatenated_initial_conditions.evaluate(0, inputs=ext_and_inputs)
    995                     .flatten()

~/Code/PyBaMM/pybamm/models/base_model.py in set_initial_conditions_from(self, solution, inplace)
    417                 try:
    418                     print(var.name)
--> 419                     final_state = solution[var.name]
    420                 except KeyError as e:
    421                     raise pybamm.ModelError(

~/Code/PyBaMM/pybamm/solvers/solution.py in __getitem__(self, key)
    355         else:
    356             # otherwise create it, save it and then return it
--> 357             self.update(key)
    358             return self._variables[key]
    359 

~/Code/PyBaMM/pybamm/solvers/solution.py in update(self, variables)
    328                     vars_casadi.append(var_casadi)
    329 
--> 330                 var = pybamm.ProcessedVariable(vars_pybamm, vars_casadi, self)
    331 
    332             # Save variable and data

~/Code/PyBaMM/pybamm/solvers/processed_variable.py in __init__(self, base_variables, base_variables_casadi, solution, warn)
     92                     else:
     93                         # Raise error for 3D variable
---> 94                         raise NotImplementedError(
     95                             "Shape not recognized for {} ".format(base_variables[0])
     96                             + "(note processing of 3D variables is not yet implemented)"

NotImplementedError: Shape not recognized for y[1:3001] (note processing of 3D variables is not yet implemented)
DavidMStraub commented 3 years ago

Can anyone reproduce this?

valentinsulzer commented 3 years ago

Yes, there are two possible fixes, either

1) edit processed variable so that 3D variables are allowed

or

2) add a hack to set_initial_conditions_from to avoid needing to call processed variable in 3D

The latter is probably easier. If the 3D variable is already a state in the system (which it will be in most cases), we can just directly call solution.y[slice] instead of solution[var.name]. The reason set_initial_conditions_from calls solution[var.name] by default is so you can initialize a model using a different model which does not have all the same states

masoodtamaddon commented 2 years ago

Hi, I was wondering if you eventually managed to solve the issue with the experiment protocol with either of the above suggestions? I didn't manage to make it work.

valentinsulzer commented 2 years ago

This has not been done yet