Issue 704 surface extrapolation

[Scottmar93]

Description

Updated the boundary value surface extrapolation to a quadratic expansion (a mathematica notebook calculating the components can be found at: https://github.com/Scottmar93/extrapolation-coefficents/tree/master). Also extended to be valid for non-uniform grids.

I haven't implemented the boundary flux equivalent as only need the surface vals to improve comparison with comsol

I have stuck an option within the boundary value symbol to turn on quadratic or linear extrapolation. Whilst it is more related to the discretization, I think this is the easiest place to put it to access the option.

Fixes #704

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.

• 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]

All looks good, and well done for getting it working on non-uniform notebooks too.
It would be cleaner to make a new class FiniteVolumeQuadraticBoundary, which inherits from FiniteVolume and only overwrites the boundary value; this would also allow comparison of linear vs quadratic boundary value to highlight the benefit of the latter

[codecov]

Codecov Report

Merging #707 into master will decrease coverage by 0.07%.
The diff coverage is 92.68%.

@@            Coverage Diff            @@
##           master    #707      +/-   ##
=========================================
- Coverage   98.18%   98.1%   -0.08%     
=========================================
  Files         175     175              
  Lines        9090    9190     +100     
=========================================
+ Hits         8925    9016      +91     
- Misses        165     174       +9

Impacted Files	Coverage Δ
...ollector/effective_resistance_current_collector.py	96.29% <100%> (ø)	⬆️
...m/models/full_battery_models/base_battery_model.py	99.62% <100%> (ø)	⬆️
pybamm/spatial_methods/zero_dimensional_method.py	100% <100%> (ø)	⬆️
pybamm/spatial_methods/scikit_finite_element.py	97.95% <100%> (+0.02%)	⬆️
pybamm/discretisations/discretisation.py	99.71% <100%> (ø)	⬆️
pybamm/simulation.py	78.94% <50%> (-0.52%)	⬇️
pybamm/spatial_methods/spatial_method.py	94.93% <91.66%> (-0.72%)	⬇️
pybamm/spatial_methods/finite_volume.py	96.98% <91.95%> (-1.85%)	⬇️

---

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 14681fb...8e2c72c. Read the comment docs.

[Scottmar93]

I have put the extrapolation forms in and added options for spatial methods. The new extrapolations work on some toy examples in the tests but they don't seem to recover the correct behaviour when put into models.

[valentinsulzer]

Thanks @Scottmar93 , looks great now!

[rtimms]

nice! looks great, thanks @Scottmar93 . are there any places in the notebook/docs which need updating so that the spatial methods are instances?