-
-
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
Plett OCP Hysteresis Model #3593
Plett OCP Hysteresis Model #3593
Conversation
pybamm/models/submodels/interface/open_circuit_potential/plett_ocp.py
Outdated
Show resolved
Hide resolved
pybamm/models/submodels/interface/open_circuit_potential/plett_ocp.py
Outdated
Show resolved
Hide resolved
pybamm/models/submodels/interface/open_circuit_potential/plett_ocp.py
Outdated
Show resolved
Hide resolved
@tinosulzer just bumping this comment again to see if you had any ideas
…On Thu, Dec 21, 2023, 1:57 PM Tanner Leo ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In pybamm/models/submodels/interface/open_circuit_potential/plett_ocp.py
<https://urldefense.com/v3/__https://github.com/pybamm-team/PyBaMM/pull/3593*discussion_r1434544378__;Iw!!AT_l0aBtzg!x3qeLaE3zT60iD8EYTIecj026S4nMsCDsWLbNxYnZOaxmcjc3D57li60JuItOpydhv2T4F2G_Lk32Sfwyq7eOQ$>
:
> + if domain_options["particle size"] == "distribution":
+ if sto_surf.domains['primary'] == f"{domain} electrode":
+ ocp_surf = ocp_surf_eq + H * h
+ else:
+ ocp_surf = ocp_surf_eq + H_x_av * h_x_av
+ else:
+ ocp_surf = ocp_surf_eq + H * h
So I have been able to get the DFN tests to pass and MPM tests to pass,
although I realized I needed to x_average the h and H variables for the
MPM, but not for the DFN with a psd. Is there any variable where I can have
that logic execute automatically?
—
Reply to this email directly, view it on GitHub
<https://urldefense.com/v3/__https://github.com/pybamm-team/PyBaMM/pull/3593*pullrequestreview-1793720164__;Iw!!AT_l0aBtzg!x3qeLaE3zT60iD8EYTIecj026S4nMsCDsWLbNxYnZOaxmcjc3D57li60JuItOpydhv2T4F2G_Lk32ScE0juZ-A$>,
or unsubscribe
<https://urldefense.com/v3/__https://github.com/notifications/unsubscribe-auth/BBQPXVIOAN7D3URF3EVUNIDYKSWEFAVCNFSM6AAAAABAII3LR2VHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMYTOOJTG4ZDAMJWGQ__;!!AT_l0aBtzg!x3qeLaE3zT60iD8EYTIecj026S4nMsCDsWLbNxYnZOaxmcjc3D57li60JuItOpydhv2T4F2G_Lk32SeB9kzSwg$>
.
You are receiving this because you authored the thread.Message ID:
***@***.***>
|
…into pr/mleot/3593
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for addressing these! Just one remaining comment about documentation
docs/source/examples/notebooks/models/differential-capacity-hysteresis-state.ipynb
Outdated
Show resolved
Hide resolved
" \"Current [A]\",\n", | ||
" \"Negative electrode secondary open-circuit potential [V]\",\n", | ||
"]\n", | ||
"pybamm.QuickPlot(\n", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nice! this model is a great addition!
pybamm/models/submodels/interface/open_circuit_potential/wycisk_ocp.py
Outdated
Show resolved
Hide resolved
pybamm/models/submodels/interface/open_circuit_potential/wycisk_ocp.py
Outdated
Show resolved
Hide resolved
pybamm/models/submodels/interface/open_circuit_potential/wycisk_ocp.py
Outdated
Show resolved
Hide resolved
pybamm/models/submodels/interface/open_circuit_potential/wycisk_ocp.py
Outdated
Show resolved
Hide resolved
pybamm/models/submodels/interface/open_circuit_potential/wycisk_ocp.py
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Happy with this, let's merge!
.diff()
operates on the symbolic expression tree, so it doesn't require a function
Thanks for walking me through it! Hopefully the next one goes smoother! |
@all-contributors please add @mleot for code and tests |
I've put up a pull request to add @mleot! 🎉 |
Nice addition 👍. The heat generation due to OCP hysteresis still isn't accounted for, is it? |
I don't think so. Do you know any references on how this should be accounted for? |
Only using terminal voltage & energy balance like in https://doi.org/10.1038/s41560-019-0410-6 ... @rtimms might be able to answer better 🙂 |
Description
Adds the hysteresis model as outlined in Wycisk 2022, and as mentioned in
#3375 and as discussed in #3376 .
This will allow for hysteresis switching to depend not on the current, but on the differential capacity change.
Fixes #3375
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: