I2304 multiple models to solver

[martinjrobins]

Description

IDAKLU solver cannot setup more than one model. We have decided to restrict the BaseSolver to only setup a single model at at time. If multiple models are passed in a RuntimeError is raised

Fixes #2304

Type of change

• Bug fix (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

Base: 99.72% // Head: 99.72% // Decreases project coverage by -0.00% ⚠️

Coverage data is based on head (e24a9fc) compared to base (c38f385).
Patch coverage: 100.00% of modified lines in pull request are covered.

Additional details and impacted files

@@             Coverage Diff             @@
##           develop    #2481      +/-   ##
===========================================
- Coverage    99.72%   99.72%   -0.01%     
===========================================
  Files          270      270              
  Lines        19487    19502      +15     
===========================================
+ Hits         19434    19448      +14     
- Misses          53       54       +1     

Impacted Files	Coverage Δ
pybamm/simulation.py	99.76% <100.00%> (-0.24%)	⬇️
pybamm/solvers/base_solver.py	100.00% <100.00%> (ø)
pybamm/solvers/jax_solver.py	96.77% <100.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

[valentinsulzer]

Thanks, looks good and sorry for slow review.
In longer-term future, it would be better if the model itself (rather than the solver) stored the setting up functions, since it seems a bit weird to restrict one model instance per solver. But I'm happy to merge this for now as it fixes the existing bug

[martinjrobins]

Thanks @tinosulzer! I'll make those fixes. FYI, I'm quite happy with one-model-one-solver approach. There is always going to be significant setup for any model-solver pairing, and it makes sense to have setup stuff done within an __init__() rather than anywhere else

[valentinsulzer]

Ok fair enough, if it isn't broken don't fix it I guess

Updates look good so happy to merge once tests pass (merging develop should do it)

[review-notebook-app]

Check out this pull request on 

See visual diffs & provide feedback on Jupyter Notebooks.

---

Powered by ReviewNB