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.

Sign up for GitHub

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

Jump to bottom

[Bug]: LLI due to SEI incorrect if loss of active material is enabled #3006

Open
DrSOKane opened this issue Jun 1, 2023 · 6 comments · May be fixed by #4869
Open

[Bug]: LLI due to SEI incorrect if loss of active material is enabled #3006

DrSOKane opened this issue Jun 1, 2023 · 6 comments · May be fixed by #4869
Assignees
@DrSOKane @RuiheLi
Labels
bug Something isn't working difficulty: medium Will take a few days in-progress Assigned in the core dev monthly meeting priority: medium To be resolved if time allows

Comments

@DrSOKane
Copy link
Contributor

DrSOKane commented Jun 1, 2023

PyBaMM Version

23.4.1

Python Version

3.8.10

Describe the bug

In order to convert SEI thickness into SEI concentration, the surface area to volume ratio a is required. However, if any of the loss of active material models are enabled, a changes over time.

Prior to 15th November 2022, a_typ is used to convert from thickness to concentration. This results in SEI growth being overestimated, with SEI growing on particles that have already been lost.

On 15th November 2022, a_typ was replaced with the time-dependent a(t). However, this is also flawed. If a particle that already had SEI on it is lost, PyBaMM considers that SEI to also be lost, which results in "Loss of lithium due to SEI [mol]" decreasing over time in some cases.

One solution could be to have separate ODEs for the various LLI variables so they are incremented at the point the lithium is lost, but that would be computationally expensive and surely there must be a better way?

Steps to Reproduce

import pybamm
import matplotlib.pyplot as plt
model = pybamm.lithium_ion.DFN({
    "particle mechanics": ("swelling only", "none"),
    "SEI": "interstitial-diffusion limited",
    "loss of active material": ("stress-driven", "none")
})
param = pybamm.ParameterValues("OKane2022")
param.update({
    "Inner SEI reaction proportion": 0.5,
    "Initial inner SEI thickness [m]": 2.5e-09,
    "Initial outer SEI thickness [m]": 2.5e-09,
    "Ratio of lithium moles to SEI moles": 2.0,
})
var_pts = {"x_n": 20, "x_s": 20, "x_p": 20, "r_n": 30, "r_p": 30}
exp = pybamm.Experiment(["Discharge at 1C until 2.5 V", "Charge at 0.3C until 4.2 V", "Hold at 4.2 V until C/100"])
sim = pybamm.Simulation(model, parameter_values=param, experiment=exp)
sol = sim.solve()
t = sol["Time [s]"].entries
Q_SEI = sol["Loss of capacity to SEI [A.h]"].entries
plt.figure()
plt.plot(t,Q_SEI)
plt.show()

Relevant log output

No response
@DrSOKane DrSOKane added the bug Something isn't working label Jun 1, 2023
@valentinsulzer valentinsulzer added difficulty: medium Will take a few days priority: medium To be resolved if time allows labels Jun 12, 2023
@valentinsulzer
Copy link
Member

Related to our earlier discussion of the problem of infinite loops when trying to define the SEI model in terms of concentration: the problem is that the get_coupled_variables function in the SEI model is doing too much, so one solution is to add a new submodel whose only role is to implement a get_coupled_variables that converts concentrations to thicknesses. It's ok for a submodel to only implement get_coupled_variables, and not the other functions: here is one example

@RuiheLi
Copy link
Contributor

RuiheLi commented Jul 5, 2023

Related to our earlier discussion of the problem of infinite loops when trying to define the SEI model in terms of concentration: the problem is that the get_coupled_variables function in the SEI model is doing too much, so one solution is to add a new submodel whose only role is to implement a get_coupled_variables that converts concentrations to thicknesses. It's ok for a submodel to only implement get_coupled_variables, and not the other functions: here is one example

In this case, we would use concentration as the basic variables then? Based on that, shall we seperate "SEI concentration on active materials", and "SEI concentration on de-active materials", i.e., similar to the lithium plating model?

@valentinsulzer
Copy link
Member

Maybe, but it might make more sense to have a single "SEI concentration" variable and then split proportionally to LAM. Can you share the equations?

@RuiheLi
Copy link
Contributor

RuiheLi commented Jul 5, 2023 •
edited
Loading

image
I am still thinking, haven't come up with the full equations yet. As presented in the screenshot, I am thinking to have a SEI concentration on "currently active" material, and another SEI concentration on the "dead active" material, then we still solve the SEI model and growth SEI on those "currently active" materials, then based on how much LAM is lost during this time range, we "transfer" some SEI from the "currently active" material to the "dead active" material, I am now stuck on how to explcitly write the two equations for these two SEI concentrations

@RuiheLi
Copy link
Contributor

RuiheLi commented Jul 5, 2023

image
A bit clearer version

@RuiheLi
Copy link
Contributor

RuiheLi commented Jul 10, 2023 •
edited
Loading

After discussing with Simon and have a second though, I think the best solution would be to change the basic variables to SEI concentration, defined as the total SEI amount in mole over the negative electrode volume (including the pores). In that case, we won't loss track to those SEI even they grows on de-active electrode materials.
image

But then we get a proper SEI thickness, we may still need to spilt SEI on active material and SEI on "dead active" material.

@valentinsulzer valentinsulzer added the in-progress Assigned in the core dev monthly meeting label Jul 10, 2023
@brosaplanella brosaplanella moved this to In Progress in August 2023 sprint Aug 7, 2023
@brosaplanella brosaplanella mentioned this issue Jul 27, 2023
Closed
8 tasks
@brosaplanella brosaplanella moved this from In Progress to Awaiting Review in August 2023 sprint Sep 4, 2023
@DrSOKane DrSOKane linked a pull request Feb 21, 2025 that will close this issue
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@DrSOKane DrSOKane

@RuiheLi RuiheLi

Labels
bug Something isn't working difficulty: medium Will take a few days in-progress Assigned in the core dev monthly meeting priority: medium To be resolved if time allows
Projects
No open projects
August 2023 sprint
Status: Awaiting Review
Milestone
No milestone
3 participants
@valentinsulzer @DrSOKane @RuiheLi