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

Discussion: Experiment and Solve with Experiment Refactor #4840

Open
aabills opened this issue Feb 12, 2025 · 10 comments
Open

Discussion: Experiment and Solve with Experiment Refactor #4840

aabills opened this issue Feb 12, 2025 · 10 comments

Comments

@aabills
Copy link
Contributor

aabills commented Feb 12, 2025

There are a number of things that we could change about current pybamm experiments to make them more flexible and align better with battery cycling experiments. For example, it is not currently possible to dynamically change pybamm experiments based on which termination is activated (for example, it might be useful to run a final RPT if the voltage reaches some predefined lower threshold but continue cycling if not).

Also, there has been some discussion of moving this code to C++ to improve speed. This issue is intended to discuss a potential refactor of the experiments to better match cycling protocols.

Here is the minimum set of features from my side that I think we need to accomplish this:

  • "goto": go to a specific experiment step if some condition is met
  • "partial CCCV": allow "CCCV" to be one step such that a duration can be defined for the entire sequence rather than the CC and CV steps separately
  • "Step groups" is something @valentinsulzer has mentioned to me

This could be as big or small of a change as we think is necessary; if we think we can accomplish everything we want without a large rewrite, that would be great too.

tagging @valentinsulzer @MarcBerliner @brosaplanella

@valentinsulzer
Copy link
Member

valentinsulzer commented Feb 12, 2025

I thought step groups would be a possible way to implement both goto and duration limits for multiple steps e.g.

Experiment({"group 1": StepGroup([step1, step2, step3], gotos={"termination1": "group2", "termination2": "group3"}, duration="5 minutes"), "group 2": [...], "group3": [...]})

The existing Experiment would be backwards compatible as a special case with only one StepGroup with default options

Other requests for the refactor:

  • move initial soc to the experiment, instead of sim.solve
  • "variables" that propagate from step to step e.g. "charge for 20 minutes then hold that voltage" or "discharge until 20% SOC, based on capacity from the previous step"
  • make temperature and current required inputs (i.e. only run simulations via experiments, don't specify the ambient temperature, initial temperature, and current in the parameter set)
  • recording frequency based on both time and voltage intervals (e.g. record every second or every 1mV)

@valentinsulzer
Copy link
Member

  • allow "Vmin" and "Vmax" as limits (and "Imax", "Tmax", etc)?
  • specify safety limits that make the experiment stop completely?

@brosaplanella
Copy link
Member

In PyProBE they use a specific in a syntax in a yaml file (https://imperialcollegelondon.github.io/PyProBE/user_guide/writing_a_readme_file.html).

I don't think these features are built in there, but it would be nice to eventually converge into a similar format.

@valentinsulzer
Copy link
Member

PyProBE format looks neat, I like the experiment titles. Not a fan of the hardcoded GOTOs based on step number, I think it's better to just specify a number of repeats for a given group of steps

@aabills
Copy link
Contributor Author

aabills commented Feb 18, 2025

  • allow "Vmin" and "Vmax" as limits (and "Imax", "Tmax", etc)?
  • specify safety limits that make the experiment stop completely?

Adding to this, it might be nice if we could remove upper and lower voltage cutoffs from parameters and just have them be a property of the experiment -- this is more reflective of physical reality anyway (cells don't have magical voltage cutoffs)

@valentinsulzer
Copy link
Member

This would make it annoying to specify initial SOC though. I think nominal capacity and min/max voltage are all "cell metadata" which are not parameters but also you don't want to have to specify them for each experiment so the "ParameterValues" dict is where it makes the most sense to put them. Current and ambient/initial temperature on the other hand definitely should not be in the cell parameters

@aabills
Copy link
Contributor Author

aabills commented Feb 18, 2025

This would make it annoying to specify initial SOC though.

Don't we use Open-circuit voltage at 0% SOC [V] and Open-circuit voltage at 100% SOC [V] for that? And if we don't, shouldn't we?

I think nominal capacity and min/max voltage are all "cell metadata" which are not parameters but also you don't want to have to specify them for each experiment so the "ParameterValues" dict is where it makes the most sense to put them.

I like having 0 and 100 SOC in the dict and upper and lower cutoff in the experiment because it best reflects reality, and it gives the best possible number for SOC.

There are plenty of tests people run that get outside their cells defined 0-100% SOC range but stay under/over some other less conservative upper/lower cut-off voltage

@valentinsulzer
Copy link
Member

Yes you're right, let's do it that way

@MarcBerliner
Copy link
Member

recording frequency based on both time and voltage intervals (e.g. record every second or every 1mV)

@valentinsulzer are you imagining this as a post-processing step to simplify reporting or as a way to save memory during the solve? It won't be easy to integrate this within the solve itself since the solvers are model-agnostic, but I think you can reasonably do this as a post-processing step after the fact

@valentinsulzer
Copy link
Member

Yes, post-processing only

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants