-
-
Notifications
You must be signed in to change notification settings - Fork 553
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 SPMe to generalised version #4329
base: develop
Are you sure you want to change the base?
Conversation
I ran some comparison between all the SPMe models. AccuracyI compared the outputs of the models and there are no appreciable differences across SPMe implementations. That might sound a bit disappointing as we are comparing a good approximation with a slightly better approximation. In O'Regan 2022 maybe one can argue that the new model does slightly better, as there the thermodynamic factor and transfer number are also dependent on concentration, which is what the new model captures. I did similar comparison for the half-cell and it also agrees very well. Marquis 2019O'Regan 2022Solving timesHere are the solving times. It is just one occurrence, but I ran the example multiple times and they are representative. Basically, the new SPMe is not significantly slower than the all one so all good. These times are for the Marquis2019 parameter set and the full cell.
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## develop #4329 +/- ##
===========================================
- Coverage 99.42% 99.41% -0.01%
===========================================
Files 297 296 -1
Lines 22685 22618 -67
===========================================
- Hits 22554 22486 -68
- Misses 131 132 +1 ☔ View full report in Codecov by Sentry. |
Description
This issue generalises the SPMe formulation. The main contributions are
Integrated
model as it is no longer needed.Fixes #4274
Type of change
Please add a line in the relevant section of CHANGELOG.md to document the change (include PR #) - note reverse order of PR #s. If necessary, also add to the list of breaking changes.
Key checklist:
$ pre-commit run
(or$ nox -s pre-commit
) (see CONTRIBUTING.md for how to set this up to run automatically when committing locally, in just two lines of code)$ python run-tests.py --all
(or$ nox -s tests
)$ python run-tests.py --doctest
(or$ nox -s doctests
)You can run integration tests, unit tests, and doctests together at once, using
$ python run-tests.py --quick
(or$ nox -s quick
).Further checks: