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

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

Closed
DavidMStraub opened this issue Jul 15, 2021 · 6 comments
Labels
difficulty: medium Will take a few days priority: low No existing plans to resolve

Comments

@DavidMStraub
Copy link
Contributor

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
Copy link
Contributor Author

Can anyone reproduce this?

@valentinsulzer
Copy link
Member

Yes, there are two possible fixes, either

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

or

  1. 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
Copy link

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
Copy link
Member

This has not been done yet

@kratman
Copy link
Contributor

kratman commented Nov 21, 2024

Covered by another issue, so I will close this one for now

@kratman kratman closed this as completed Nov 21, 2024
@SimMLPhys
Copy link

Covered by another issue, so I will close this one for now

What is the other issue that covers this? I'm trying to keep track of when this might become available

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
difficulty: medium Will take a few days priority: low No existing plans to resolve
Projects
None yet
Development

No branches or pull requests

5 participants