Skip to content

Conversation

DrSOKane
Copy link
Contributor

Description

Previously, lithium was not conserved if both SEI growth and loss of active material were enabled. To fix this, the fundamental variable to be solved for is now SEI concentration, not thickness. This has the added bonus of simplifying the SEI on cracks model.

I closed #3171 and made a new one simply because PyBaMM has changed so much since then!

One major difference between #3171 and this PR is that this time around, I also deleted the parameter "Initial SEI thickness [m]" and replaced it with "Initial SEI concentration [mol.m-3]". I did this partly to make the initial conditions less complicated, and partly because it's easier to measure the amount of lithium consumed by formation than it is to measure or even estimate the initial thickness. I assumed a value of 38.34 [mol.m-3], based on Ramadass et al. (2004) and Safari et al. (2009). The exception is "OKane2022", for which I used the exact value (to 4 significant figures anyway!)

Fixes #3006

@DrSOKane DrSOKane requested a review from a team as a code owner February 21, 2025 19:04
Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

@DrSOKane
Copy link
Contributor Author

DrSOKane commented Feb 27, 2025

The reason the tests are failing is because none of the half-cell parameter sets have a value for "Initial SEI concentration [mol.m-2]" on the lithium metal electrode. Liu and Lu (2017) propose values for this and other SEI parameters on Li metal electrodes. I went ahead using those and added a citation for Liu and Lu to all half-cell parameter sets.

Copy link

codecov bot commented Feb 27, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 98.79%. Comparing base (d03d648) to head (69475fa).

Additional details and impacted files
@@             Coverage Diff             @@
##           develop    #4869      +/-   ##
===========================================
+ Coverage    98.77%   98.79%   +0.02%     
===========================================
  Files          320      321       +1     
  Lines        27111    27123      +12     
===========================================
+ Hits         26779    26797      +18     
+ Misses         332      326       -6     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@rtimms
Copy link
Contributor

rtimms commented Feb 28, 2025

In general, I am against adding parameters that aren't in the original paper, but since we already added a bunch of extra degradation parameters in these files it is OK to add this once too. Going forward we should be more strict on this.

@DrSOKane
Copy link
Contributor Author

In general, I am against adding parameters that aren't in the original paper, but since we already added a bunch of extra degradation parameters in these files it is OK to add this once too. Going forward we should be more strict on this.

The only difference is that Liu and Lu use an initial thickness of 10 nm as opposed to 5 nm for Safari et al. I wanted to check if the initial thickness was substantially different for planar electrodes, and it turns out it's not. So I could calculate an interfacial concentration from 5 nm instead of 10 nm, then remove the extra reference.

I don't know why the notebook is failing, it works fine when I run it!

@rtimms
Copy link
Contributor

rtimms commented Apr 2, 2025

@DrSOKane is this ready to be reviewed?

@DrSOKane
Copy link
Contributor Author

DrSOKane commented Apr 2, 2025

@DrSOKane is this ready to be reviewed?

Yes please! All the commits since removing the reference to Liu and Lu are just updated from develop branch

@DrSOKane DrSOKane mentioned this pull request Jul 3, 2025
8 tasks
CHANGELOG.md Outdated
Comment on lines 84 to 85
- Changed fundamental variable for all SEI models from thickness to concentration ([#4869](https://github.com/pybamm-team/PyBaMM/pull/4869))
- Replaced parameter `"Initial SEI thickness [m]"` with `"Initial SEI concentration [mol.m-3]"` ([#4869](https://github.com/pybamm-team/PyBaMM/pull/4869))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a note for later, this needs to be moved up to a current release

Copy link
Contributor

@rtimms rtimms left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks @DrSOKane looks good, just a few comments


gamma_0 = pybamm.Parameter("Dead lithium decay constant [s-1]")
L_sei_0 = pybamm.Parameter("Initial SEI thickness [m]")
L_sei_0 = pybamm.Scalar(5e-9)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this consistent if the initial SEI concentration changes, or should it be derived from that?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's a tricky one, because the thickness and concentration are coupled via the surface area to volume ratio, which is different for every cell. For every cell except OKane2022, I derive the initial concentration to be consistent with that in the original Safari et al. (2009) paper. For OKane2022, the SEI is explicitly included, so I chose a value that results in a thickness of 5 nm exactly.

Safari et al. do not consider dead lithium, so I thought it more appropriate to stick with the value from O'Kane et al. Unfortunately, my dead lithium model has never been validated, so a factor of two difference is really not going to matter! The function is only there at all as proof of concept for pure graphite, as opposed to graphite+SiOx.


gamma_0 = pybamm.Parameter("Dead lithium decay constant [s-1]")
L_sei_0 = pybamm.Parameter("Initial SEI thickness [m]")
L_sei_0 = pybamm.Scalar(5e-9)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

as above

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It was difficult deciding whether to make the initial condition a concentration or thickness. Most of the academic literature reports thickness, with Li et al. 2025 being a notable exception. But for industry, concentration will be better if - and this is a big if - the formation losses are known, because the formation loss is the initial value.

@DrSOKane
Copy link
Contributor Author

Hi everyone,

I tried using the new framework with composite parameter sets and found a problem: how do you distribute the SEI between the two phases? Most PyBaMM models tend towards the two phases having equal SEI thickness in the long term, so I decided that it makes more sense to revert to giving the initial condition as thickness, but keep concentration as the fundamental variable on the back end.

To make up for this, I added a notebook that explains how to update PyBaMM parameter sets to account for any formation or storage losses a user may have measured.

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

Successfully merging this pull request may close these issues.

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

3 participants