Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Discontinuities get detected beyond the specified time interval =>IndexError #941

Closed
YannickNoelStephanKuhn opened this issue Apr 4, 2020 · 1 comment · Fixed by #945
Assignees

Comments

@YannickNoelStephanKuhn
Copy link
Contributor

Describe the bug
If the solver detects discontinuities beyond the timepoints "t_eval" as in "solver.solve(model, t_eval)", it will try to insert them at the end, throwing an IndexError when accessing the index after the last entry in "t_eval".

To Reproduce
Steps to reproduce the behaviour:
0. Optional: pybamm.set_logging_level("INFO") to check if discontinuities got detected.

  1. Load up a default set-up of a DFN-model and CasADi-solver.
  2. Specify a charge pulse profile like so:
    pulse_currents = [current_0, current_1]
    pulse_timespans = [[t_0_l, t_0_r], [t_1_l, t_1_r]]
    def current_function(time):
    return_value = 0
    for current, timespan in zip(pulse_currents, pulse_timespans):
    return_value = return_value + current * ((timespan[0] < time) * (time < timespan[1]))
    return return_value
    parameters["Current function [A]"] = current function
  3. Set a short time interval: t_eval = np.linspace(t_0_l - eps, t_0_r, 100) with eps>0.
  4. Get the IndexError:
    File "/usr/local/lib/python3.7/dist-packages/pybamm/solvers/base_solver.py", line 556, in solve
    if dtime - eps < t_eval_dimensionless[dindex] < dtime + eps:
    IndexError: index 104 is out of bounds for axis 0 with size 104

Expected behaviour
If a discontinuity lies beyond the requested time interval "t_eval", it should be treated as if it wouldn't exist. For efficiency and stability, there shouldn't be any checks for discontinuities beyond "t_eval".

@valentinsulzer
Copy link
Member

So this should just be a case of removing any discontinuities that are greater than t_eval? For example, just after we remove any identical discontinuities?

@martinjrobins martinjrobins self-assigned this Apr 6, 2020
martinjrobins added a commit that referenced this issue Apr 6, 2020
martinjrobins added a commit that referenced this issue Apr 6, 2020
martinjrobins added a commit that referenced this issue Apr 7, 2020
#941 filter out discon events after the end of the solver time
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants