Issue 976 casadi extrapolate warning

[brosaplanella]

Description

When an interpolant is created from data, a new type of event (INTERPOLANT_EXTRAPOLATION) is added to ensure that we can catch when extrapolation occurs. This is particularly important in CasADI as you get a lot of warnings that are not very insightful.

I haven't managed to remove all these warnings as, before hitting the event and raising the error, the integration step where extrapolation happens takes place. However, I don't know how to get rid of them if we can't catch the CasADI warnings.

Fixes #976

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

[rtimms]

thanks @brosaplanella looks good! my only question is should we also raise a warning/error when extrapolation occurs with the other solvers?

[codecov]

Codecov Report

Merging #1315 (4a64cc6) into develop (0926080) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff            @@
##           develop    #1315   +/-   ##
========================================
  Coverage    98.10%   98.11%           
========================================
  Files          272      272           
  Lines        15290    15339   +49     
========================================
+ Hits         15001    15050   +49     
  Misses         289      289           

Impacted Files	Coverage Δ
pybamm/solvers/jax_bdf_solver.py	98.13% <ø> (ø)
pybamm/models/event.py	90.00% <100.00%> (+0.52%)	⬆️
pybamm/parameters/parameter_values.py	99.35% <100.00%> (+0.01%)	⬆️
pybamm/solvers/base_solver.py	99.09% <100.00%> (+0.05%)	⬆️
pybamm/solvers/casadi_solver.py	99.52% <100.00%> (+0.04%)	⬆️
pybamm/solvers/idaklu_solver.py	90.26% <100.00%> (ø)
pybamm/solvers/jax_solver.py	98.30% <100.00%> (ø)
pybamm/solvers/scikits_dae_solver.py	98.46% <100.00%> (ø)
pybamm/solvers/scikits_ode_solver.py	88.75% <100.00%> (ø)
pybamm/solvers/scipy_solver.py	100.00% <100.00%> (ø)

---

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 0926080...4a64cc6. Read the comment docs.

[valentinsulzer]

Looks good to me, maybe @rtimms wants to take another look though

[rtimms]

Looks good to me, thanks!

I was wondering if we should make all the solvers give at least a warning when you extrapolate. At the moment e.g. the scipy solver only stops for the terminal events (as expected), but should we also check and warn if extrapolation has occurred so that users are aware and can check if they trust the results. Maybe this should be a separate issue though.

[brosaplanella]

@rtimms Sorry, you are totally right. I had in mind adding the warning for the other solvers, but I wanted first to check if this made sense or not.

I have tried what happens with IDAKLU and Scikits DAE solvers and they take a very long time when there is extrapolation. For example, if you try this with my_solver equal to the solver of your choice you will see it (can you check in your laptops please, just in case?)

model = pybamm.lithium_ion.DFN()

param = pybamm.ParameterValues(chemistry=pybamm.parameter_sets.Chen2020)
ci = param["Initial concentration in positive electrode [mol.m-3]"]
param["Initial concentration in positive electrode [mol.m-3]"] = 0.8 * ci

sim = pybamm.Simulation(
    model,
    parameter_values=param,
    solver=my_solver,
)

sim.solve()

In this case, should I add the warning or should I add an error? Note that I will implement the warnings anyway in issue #989 so we can change our minds later.

[rtimms]

IMO a warning would be good. At least that way you get the solution back and can make sure it makes sense (and check how much you extrapolated by). Happy to leave this for another issue though and get this PR merged.

[brosaplanella]

Cool, then I will add the warnings on this PR.

[rtimms]

thanks @brosaplanella !