Issue 546 models

[valentinsulzer]

Description

Add some 1+1D models for lead-acid

Fixes #546

Type of change

• New feature (non-breaking change which adds functionality)

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
• Any dependent changes have been merged and published in downstream modules

[codecov]

Codecov Report

❗ No coverage uploaded for pull request base (issue-546-2plus1-lead-acid@ec5b99a). Click here to learn what that means.
The diff coverage is 71.9%.

@@                      Coverage Diff                      @@
##             issue-546-2plus1-lead-acid     #564   +/-   ##
=============================================================
  Coverage                              ?   96.47%           
=============================================================
  Files                                 ?      151           
  Lines                                 ?     7433           
  Branches                              ?        0           
=============================================================
  Hits                                  ?     7171           
  Misses                                ?      262           
  Partials                              ?        0

Impacted Files	Coverage Δ
...well/diffusion/leading_stefan_maxwell_diffusion.py	100% <ø> (ø)
...ttery_models/lithium_ion/base_lithium_ion_model.py	100% <ø> (ø)
pybamm/solvers/ode_solver.py	100% <ø> (ø)
...odels/oxygen_diffusion/leading_oxygen_diffusion.py	100% <ø> (ø)
...eading_surface_form_stefan_maxwell_conductivity.py	100% <ø> (ø)
...ace/diffusion_limited/leading_diffusion_limited.py	100% <ø> (ø)
pybamm/spatial_methods/finite_volume.py	98.72% <100%> (ø)
...amm/models/submodels/current_collector/__init__.py	100% <100%> (ø)
pybamm/models/reaction_diffusion.py	100% <100%> (ø)
...bamm/models/full_battery_models/lithium_ion/dfn.py	100% <100%> (ø)
... and 29 more

---

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 ec5b99a...4e49c2f. Read the comment docs.

[codecov]

Codecov Report

Merging #564 into master will increase coverage by 0.18%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master     #564      +/-   ##
==========================================
+ Coverage   98.05%   98.24%   +0.18%     
==========================================
  Files         151      151              
  Lines        7319     7393      +74     
==========================================
+ Hits         7177     7263      +86     
+ Misses        142      130      -12

Impacted Files	Coverage Δ
...well/diffusion/leading_stefan_maxwell_diffusion.py	100% <ø> (ø)	⬆️
...ttery_models/lithium_ion/base_lithium_ion_model.py	100% <ø> (+15.78%)	⬆️
pybamm/solvers/ode_solver.py	100% <ø> (ø)	⬆️
...eading_surface_form_stefan_maxwell_conductivity.py	100% <ø> (ø)	⬆️
...odels/oxygen_diffusion/leading_oxygen_diffusion.py	100% <ø> (ø)	⬆️
...ace/diffusion_limited/leading_diffusion_limited.py	100% <ø> (ø)	⬆️
pybamm/spatial_methods/finite_volume.py	98.72% <100%> (ø)	⬆️
...amm/models/submodels/current_collector/__init__.py	100% <100%> (ø)	⬆️
pybamm/models/reaction_diffusion.py	100% <100%> (ø)	⬆️
...bamm/models/full_battery_models/lithium_ion/dfn.py	100% <100%> (ø)	⬆️
... and 38 more

---

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 c3a4a8c...fbbf1ed. Read the comment docs.

[rtimms]

looks great thanks @tinosulzer !

[rtimms]

for 1D do we expect the i_boundary_cc to be current_with_time or current_with_time/cc_area ? Not for this issue, but I'm not sure if it would be more user friendly to provide cell capacity in the csv files and then set the typical current to be the 1C discharge current and C-rate to be 1 by default (obviously the user can adjust these later)

[valentinsulzer]

In 1D I think it should be (dimensionless) current_with_time, which is the same as dimensionless current_density_with_time but not the same as current_with_time/cc_area.
Don't completely understand the point about 1C discharge currents?

[rtimms]

ah ok. for the 1C thing I was just thinking that for users it might seem strange to provide the typical current in a parameter file, and may be more natural to give the cell capacity as the parameter, which then sets the default typical current (to the 1C discharge current). maybe this is a bit of a non-issue though

[valentinsulzer]

Ok - I guess ideal scenario is you can do either and it updates the other. Isn't most battery data current vs voltage rather than Crate vs voltage though?

[rtimms]

Even if you are reading in current vs voltage data you still only set one typical current used to non-dimensionlise. As you say, setting either would be best. In any case, I think the default current shouldn't be set to 1A and should instead be set based on the cell capacity (if provided). Then if you are going to be loading in current vs voltage data you might want to change this to a typical current based on your data and then the C-rate changes accordingly (I feel like I'm not explaining this very well haha)