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

Update Ecker2015 solid-phase diffusivity functions to more closely match the paper #4141

Merged
merged 8 commits into from
Jun 11, 2024

Conversation

DrSOKane
Copy link
Contributor

@DrSOKane DrSOKane commented Jun 5, 2024

The solid-phase diffusivity functions in Ecker2015 and Ecker2015_graphite_halfcell are updated to better match the functions in the original paper. A new citation Yuan2023, for which these parameters were used, is also added.

Advantages:

  • The new functions reproduce phase separation behaviour in a similar way to the Cahn-Hilliard equations
  • With them, the results in Yuan2023 can be reproduced.

Disadvantages:

  • This level of physical accuracy comes at a noticeable computational cost, comparable with that of ORegan2022
  • The results in Richardson2019 are no longer reproduced. However, that paper still made a major contribution towards our understanding of the Ecker2015 parameters and is therefore still cited.

@rtimms
Copy link
Contributor

rtimms commented Jun 5, 2024

My preference would be to add a new Yuan2023 parameter set so that the Ecker results don't change from version to version.

Copy link
Member

@valentinsulzer valentinsulzer left a comment

Choose a reason for hiding this comment

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

What's the formula for D_s_n and D_s_p actually used by Ecker? I can't find it in Part I or Part II.

@@ -47,6 +48,7 @@

## Breaking changes

- Updated solid-phase diffusivity functions in `Ecker2015` and `Ecker2015_graphite_halfcell` parameter sets to better match the original papers ([#4141](https://github.com/pybamm-team/PyBaMM/pull/4141))
Copy link
Contributor

Choose a reason for hiding this comment

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

This should probably be under only "breaking changes" or "features" instead of both

@kratman
Copy link
Contributor

kratman commented Jun 5, 2024

@DrSOKane Is this one fixed by this change as well?

#2152

@DrSOKane
Copy link
Contributor Author

DrSOKane commented Jun 6, 2024

What's the formula for D_s_n and D_s_p actually used by Ecker? I can't find it in Part I or Part II.

They don't give a formula, only data. It's possible that they used an interpolation between the data points. LiionDB has this data (I put it there!) so an alternative approach would be to use that.

@DrSOKane DrSOKane merged commit 510fca7 into pybamm-team:develop Jun 11, 2024
6 of 24 checks passed
@kratman
Copy link
Contributor

kratman commented Jun 11, 2024

@DrSOKane This PR was marked as merged because the commits were also present in #4168

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.

4 participants