Electrolyte overpotentials

[brosaplanella]

Description

Added the electrolyte overpotential and ohmic losses to the full_conductivity model.

Fixes #1349

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: $ flake8
• All tests pass: $ python run-tests.py --unit
• The documentation builds: $ cd docs and then $ make clean; make html

You can run all three at once, using $ python run-tests.py --quick.

Further checks:

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

[valentinsulzer]

We should also be able to replace the get in these lines with a standard dictionary read

PyBaMM/pybamm/models/full_battery_models/base_battery_model.py

Line 851 in fa076b8

eta_e_av = self.variables.get("X-averaged electrolyte ohmic losses", 0)

[codecov]

Codecov Report

Merging #1350 (fa076b8) into develop (f8b58f2) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff            @@
##           develop    #1350   +/-   ##
========================================
  Coverage    98.15%   98.15%           
========================================
  Files          272      272           
  Lines        15577    15578    +1     
========================================
+ Hits         15289    15290    +1     
  Misses         288      288           

Impacted Files	Coverage Δ
pybamm/spatial_methods/spectral_volume.py	98.11% <100.00%> (+0.01%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f8b58f2...322c432. Read the comment docs.

[brosaplanella]

@tinosulzer I have tried what you suggested but it causes issues with the surface form equations. Because they are defined on each electrode and separator separately it is not clear to me where the overpotentials (which are global) should be defined. One way would be to do it where the DFN (and similarly for the others) is defined but that seems an unnecessary repetition of the code.

Alternatively, I can leave the get there and open another issue for the surface form.

[rtimms]

Could you add an extra submodel in surface_potential_form that is used for calculating the overpotentials? Not ideal, but not entirely dissimilar to e.g. how we have CurrentForInverseButlerVolmer.

Alternatively, could you make calculating the overpotentials a new method in BaseElectrolyteConductivity that has a try/except in to get the variables it needs. Then the surface model on each domain would call it in turn - calculating the overpotentials would be skipped until all the necessary variables are defined. Not sure this is a very nice solution though...

EDIT: looks like you already fixed this, sorry!

[rtimms]

Thanks @brosaplanella ! If you wanted to avoid repeated code you could add a method _get_overpotentials which you can call from both _get_whole_cell_variables in the base model and get_coupled_variables in the full model.