Issue 741 neumann bc

[rtimms]

Description

Changes finite volumes to use the exact value of the gradient at the cell edges if given by a Neumann value, instead of adding a ghost node.

Fixes #741

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

[codecov]

Codecov Report

Merging #748 into master will increase coverage by 0.01%.
The diff coverage is 97.75%.

@@            Coverage Diff             @@
##           master     #748      +/-   ##
==========================================
+ Coverage   98.35%   98.36%   +0.01%     
==========================================
  Files         179      179              
  Lines        9810     9871      +61     
==========================================
+ Hits         9649     9710      +61     
  Misses        161      161

Impacted Files	Coverage Δ
pybamm/spatial_methods/finite_volume.py	97.47% <97.75%> (+0.45%)	⬆️

---

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 2ca2598...5b5b224. Read the comment docs.

[valentinsulzer]

Looks good, I haven't checked the linear algebra but if all existing tests pass then great!
Out of interest though, shouldn't the previous formulation have given fluxes exactly? There was no extrapolation for those?

[valentinsulzer]

where is domain name adjusted?

[rtimms]

ah thanks domain name is adjusted in the lines above, but i forgot to move the comment. maybe i misunderstood what was happening before. it looked like we added a ghost node assuming the function was linear there with slope given by the Neumann bc? that was probably consistent with the scheme anyway, and was giving the correct result. it doesn't look like doing things the way i've implemented actually makes any different other than we no longer unnecessarily add extra ghost nodes

[valentinsulzer]

Before it was adding a ghost node in such a way that (ghost_node - last_internal_node) / dx = flux, but good to avoid adding unnecessary ghost nodes