Repository navigation
fix: broadcast the average SEI film overpotential with a size distribution - #5892
Open
shaurya20335 wants to merge 4 commits into
Open
shaurya20335 wants to merge 4 commits into
shaurya20335 wants to merge 4 commits into
Conversation
…ution With "SEI film resistance": "average", eta_sei is the x-averaged film drop on the current collector. With "particle size": "distribution" resolved through the electrode (SPMe, DFN, Newman-Tobias), eta_r lives on the particle-size domain with the electrode as its secondary domain, so adding eta_sei raised DomainError: children must have same or empty domains, not ['negative particle size'] and ['current collector']. Broadcast eta_sei onto the electrode in that case. Models that already built are unchanged. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Contributor
|
Looks correct, thanks. I checked it together with #5841 (SEI film heating) and #5875: every size-distribution model builds with no, average or distributed film, and with #5841 the Heads-up on #5818, which deprecates relying on options set from other options (the test suite turns that warning into an error): |
This was referenced Oct 12, 2026
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Contributor
Author
4 of 5 tasks
…m-size-distribution # Conflicts: # CHANGELOG.md
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
With
"SEI film resistance": "average"and"particle size": "distribution", SPMe, DFN and Newman-Tobias fail to build:In
BaseKinetics, the average film dropeta_sei = -j_tot_av * L_sei_av * R_seilives on the current collector. When the size distribution is resolved through the electrode,eta_ris on the particle-size domain with the electrode as its secondary domain, soeta_r += eta_seicannot broadcast across the missing electrode level. MPM is not affected because itseta_ralready has the current collector as its secondary domain, and"distributed"is not affected because itseta_seiis on the electrode.eta_seiis now broadcast onto the electrode in that case. Nothing changes for models that already built: MPM and DFN without a distribution give bit-identical voltages with the average film.With the fix (OKane2022,
"SEI": "constant", 50 nm film, 5 A, voltage at 600 s):The average and distributed films differ by about 1 mV with a size distribution, as they do without one (1.5 mV for the DFN).
I found this while checking #5841 across model options.
Type of change
Bug fix, with an entry under
## Bug fixesin CHANGELOG.md.Important checks:
test_well_posed_psd_sei_film_resistance(unit, in the shared lithium-ion base tests) builds each model with a size distribution and both film options. It fails on main for SPMe, DFN and Newman-Tobias and passes with the fix.