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

More informative error extrapolation drive cycle #3612

Closed
brosaplanella opened this issue Dec 12, 2023 · 6 comments · Fixed by #3756
Closed

More informative error extrapolation drive cycle #3612

brosaplanella opened this issue Dec 12, 2023 · 6 comments · Fixed by #3756
Assignees
Labels
difficulty: easy A good issue for someone new. Can be done in a few hours priority: medium To be resolved if time allows

Comments

@brosaplanella
Copy link
Sponsor Member

At the moment, if a drive cycle starts at t=1 rather than t=0 it throws a rather uninformative error (see below)

fe445cd2-3667-45a0-8a78-0807173bf6c3

The easiest way to address this would be:

  • Give the interpolant a meaningful name at creation so at least it is clear which interpolant is causing the error
    self.value = pybamm.Interpolant(
    t, y, pybamm.t - pybamm.InputParameter("start time")
    )
  • (Optionally) Perform a check and if drive cycle starts at t>0 throw a different, more specific error (or assume it's zero until first provided value, but I personally think this is dangerous).
  • Add somewhere in the docs that drive cycles must start at t=0, added to Improve documentation (ongoing) #3392 already.
@brosaplanella brosaplanella added difficulty: easy A good issue for someone new. Can be done in a few hours priority: medium To be resolved if time allows labels Dec 12, 2023
@jeromtom
Copy link
Contributor

Would Drive cycle start time be an apt name?

@brosaplanella
Copy link
Sponsor Member Author

Would Drive cycle start time be an apt name?

For the drive cycle name? I would call it something like "Current drive cycle" (or power/voltage whatever drives it).

@R-Yash
Copy link
Contributor

R-Yash commented Jan 21, 2024

@brosaplanella I want to work on this issue. Is it still open?

@brosaplanella
Copy link
Sponsor Member Author

@jeromtom are you working on it? Otherwise @R-Yash can take it

@jeromtom
Copy link
Contributor

@brosaplanella, I had worked on this but could not commit the changes due to a fault in my environment setup.
@R-Yash, you can refer to the following to fix it.
image
image

@R-Yash
Copy link
Contributor

R-Yash commented Jan 22, 2024

Thanks @jeromtom

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
difficulty: easy A good issue for someone new. Can be done in a few hours priority: medium To be resolved if time allows
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants