Current-driven LAM

[brosaplanella]

Description

Implement current-driven LAM submodel. At the moment it cannot be combined with the other ones, but that's because I wasn't sure how we should set the options (i.e. single string, list of strings...). Happy to leave it as is for the time being or to implement a fix if we agree on a format.

Fixes #3252

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.

• New feature (non-breaking change which adds functionality)
• Optimization (back-end change that speeds up the code)
• Bug fix (non-breaking change which fixes an issue)

Key checklist:

• No style issues: $ 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)
• All tests pass: $ python run-tests.py --all (or $ nox -s tests)
• The documentation builds: $ 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:

• Code is commented, particularly in hard-to-understand areas
• Tests added that prove fix is effective or that feature works

[codecov]

Codecov Report

Patch coverage: 100.00% and no project coverage change.

Comparison is base (ea7e00c) 99.71% compared to head (73ea11a) 99.71%.
Report is 27 commits behind head on develop.

Additional details and impacted files

@@           Coverage Diff            @@
##           develop    #3253   +/-   ##
========================================
  Coverage    99.71%   99.71%           
========================================
  Files          248      248           
  Lines        18824    18871   +47     
========================================
+ Hits         18770    18817   +47     
  Misses          54       54           

Files Changed	Coverage Δ
...m/models/full_battery_models/base_battery_model.py	99.76% <ø> (ø)
.../submodels/active_material/loss_active_material.py	100.00% <100.00%> (ø)
pybamm/parameters/lithium_ion_parameters.py	100.00% <100.00%> (ø)

... and 9 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[valentinsulzer]

Can you add an integration test as well?

[brosaplanella]

Can you add an integration test as well?

For the integration tests I need parameter values, but I don't have any (need to check the Safari & Delacourt paper, but not sure they will translate). Shall I just some dummy parameters (i.e. made up) and define them in place for the integration tests? I want to update the notebook anyway, so I will come up with some parameters for that too.

[valentinsulzer]

Yes dummy parameters are fine

[review-notebook-app]

Check out this pull request on 

See visual diffs & provide feedback on Jupyter Notebooks.

---

Powered by ReviewNB

[brosaplanella]

Requesting also @DrSOKane review as I made some changes to the LAM submodels notebook and I believe he was the one who wrote it.

[DrSOKane]

This new LAM option implements the empirical model of Delacourt and Safari (2012) in PyBaMM. As it is a purely empirical model, I don't see much point in making it compatible with the other LAM models. However, in both the example and the tests, only the linear term of their equation ([26] in Reniers, Mulder and Howey 2019) is included and the nonlinear term is left out. Can you try including the nonlinear term and seeing what happens?

[brosaplanella]

@all-contributors add @DrSOKane for pull request review

[allcontributors]

@brosaplanella

I've put up a pull request to add @DrSOKane! 🎉

[valentinsulzer]

Question for @brosaplanella and @DrSOKane : should the minus sign for the current-driven LAM term be in the model or in the parameter?

[brosaplanella]

I agree, but thought it would be better to stick to the notation in Reniers et al