#5198 update definition of hysteresis decay rate

[rtimms]

Description

Updates the definition of the hysteresis decay rate to be a "true" decay rate. This is a breaking change for the OneStateHysteresisOpenCircuitPotential and OneStateDifferentialCapacityHysteresisOpenCircuitPotential submodels, as described below.

Updated model formulations

OneStateHysteresisOpenCircuitPotential

This PR introduces an additional factor of 2 in the ODE for the hysteresis state variable. To retain the same behaviour, the parameter $\gamma$ (e.g. "Secondary: Negative particle lithiation hysteresis decay rate") should be multiplied by 2. The governing equations are detailed below.

OneStateHysteresisOpenCircuitPotential -- v25.8 and below

The hysteresis state variable $h(z,t)$ is governed by the following ODE:

$$ \frac{dh(z,t)}{dt} = \gamma \left( \frac{i_{\text{vol}}}{F , c_{\text{max}} , \epsilon}\right) \left(1 - \text{sgn}(i_{\text{vol}}) , h(z,t)\right).$$

OneStateHysteresisOpenCircuitPotential -- this PR onwards

The hysteresis state variable $h(z,t)$ is governed by the following ODE:

$$ \frac{dh(z,t)}{dt} = \gamma \left( \frac{i_{\text{vol}}}{F , c_{\text{max}} , \epsilon}\right) \frac{\left(1 - \text{sgn}(i_{\text{vol}}) , h(z,t)\right)}{2},$$

OneStateDifferentialCapacityHysteresisOpenCircuitPotential

This PR reformulates the model to use the same governing equation as the OneStateHysteresisOpenCircuitPotential. This introduces a significant change in the interpretation of the parameter $\Gamma$. To retain the same behaviour, the parameter $\Gamma$ (e.g. "Secondary: Negative particle hysteresis decay rate") should be multiplied by $2 / (n_{\text{electrodes}}L_{\text{k}}L_{\text{y}}L_{\text{z}}a_{\text{k}} / 3600)$. Where $L_{\text{k}}$ is the electrode thickness, $L_{\text{y}}$ is the electrode width, $L_{\text{z}}$ is the electrode height, and $a_{\text{k}}$ is the surface area per unit volume. The parameter $n_{\text{electrodes}}$ is the number of electrodes in parallel (see #5198 for discussion of a bug in v25.8).

The governing equations are detailed below.

OneStateDifferentialCapacityHysteresisOpenCircuitPotential -- v25.8 and below

The hysteresis state variable $h(z,t)$ is governed by the following ODE:

$$ \frac{dh(z,t)}{dt} = \left(\frac{\gamma \cdot i_{\text{surf}}}{Q_{\text{cell}}}\right) \left(1 - \text{sgn}(i_{\text{surf}}) , h(z,t)\right), $$

with

$$ \gamma(z) = \Gamma(z) \cdot \frac{1}{\left(C_{\text{diff}}\left(z\right)\right)^{x}}.$$

and $C_{\text{diff}}(z) = \frac{dQ}{dU_{\text{eq}}(z)}$

OneStateDifferentialCapacityHysteresisOpenCircuitPotential -- this PR onwards

The hysteresis state variable $h(z,t)$ is governed by the following ODE:

$$ \frac{dh(z,t)}{dt} = \gamma \left( \frac{i_{\text{vol}}}{F , c_{\text{max}} , \epsilon}\right) \frac{\left(1 - \text{sgn}(i_{\text{vol}}) , h(z,t)\right)}{2},$$

with

$$ \gamma(z) = \Gamma(z) \cdot \left(\frac{C_{\text{diff}}\left(z\right)}{C_{\text{ref,vol}}}\right)^{-x}.$$

and $C_{\text{diff}}(z) = \frac{dQ}{dU_{\text{eq}}(z)}$, $C_{\text{ref,vol}} = 1$.

Fixes #5198

Type of change

Please add a line in the relevant section of CHANGELOG.md to document the change (include PR #)

Important checks:

Please confirm the following before marking the PR as ready for review:

• No style issues: nox -s pre-commit
• All tests pass: nox -s tests
• The documentation builds: nox -s doctests
• Code is commented for hard-to-understand areas
• Tests added that prove fix is effective or that feature works

[review-notebook-app]

Check out this pull request on 

See visual diffs & provide feedback on Jupyter Notebooks.

---

Powered by ReviewNB

[rtimms]

@ejfdickinson I'd appreciate a second look at this to make sure my edits and conversions are correct

[codecov]

Codecov Report

❌ Patch coverage is 92.10526% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 98.81%. Comparing base (c550680) to head (e44ccfa).

Files with missing lines	Patch %	Lines
...amm/models/full_battery_models/lithium_ion/util.py	85.00%	3 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff             @@
##           develop    #5217      +/-   ##
===========================================
+ Coverage    98.79%   98.81%   +0.01%     
===========================================
  Files          320      320              
  Lines        27100    27093       -7     
===========================================
- Hits         26774    26771       -3     
+ Misses         326      322       -4     

☔ 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.

[ejfdickinson]

Will do @rtimms but not necessarily straight away - chase me later next week if still no action.

[rtimms]

@ejfdickinson just a reminder to take a look at this, thanks!

[ejfdickinson]

@rtimms Will try to do so this afternoon, sorry for delay!

[rtimms]

thanks, appreciate it!

[ejfdickinson]

@rtimms From a reading review, this seems correct to me, in the equations and implementation. I flagged some minor typos and style issues.

I haven't attempted to test the PR against any of our in-house code, as this will take longer to configure and I don't have a good opportunity. If you think this can sit open another week, and you think that the existing PyBaMM tests need support for safety, I could ask @katielukow-AE to support with this.

[rtimms]

Thanks for the review. Yes, I think it would be a helpful sanity check if @katielukow-AE is able to support with testing this further, thanks!

[aabills]

seems fine to me 

[aabills]

pls fix style and tests