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

Reformat casadi solver again #1082

Closed
2 of 4 tasks
valentinsulzer opened this issue Jun 25, 2020 · 5 comments · Fixed by #1450
Closed
2 of 4 tasks

Reformat casadi solver again #1082

valentinsulzer opened this issue Jun 25, 2020 · 5 comments · Fixed by #1450
Assignees

Comments

@valentinsulzer
Copy link
Member

valentinsulzer commented Jun 25, 2020

Another reformatting of the casadi solver is needed with

  • Better automatic choosing of dt_max (dt_max = 0.1 * min(timescale, t_final)
  • Generating the integrator once with [0,1] and rescaling time - "safe without grid" mode
  • Include the event time and state in the solution being returned - done in Return event state #1300
  • Look into a smarter way of dealing with events ("The alternative is to try to be smarter about predicting when events will occur. At constant current you can see most of our events coming from a long way away (i.e. they are pretty much linear, definitely monotonic, in time). So you could envisage an algorithm where after each step you look at the gradient of the event function, look at where the function would cross zero, and choose the size of your next step based on that.")

Anything else?

@valentinsulzer valentinsulzer self-assigned this Jun 26, 2020
valentinsulzer added a commit that referenced this issue Jun 28, 2020
valentinsulzer added a commit that referenced this issue Jun 29, 2020
valentinsulzer added a commit that referenced this issue Jun 29, 2020
valentinsulzer added a commit that referenced this issue Jun 30, 2020
valentinsulzer added a commit that referenced this issue Jul 2, 2020
valentinsulzer added a commit that referenced this issue Jul 2, 2020
valentinsulzer added a commit that referenced this issue Jul 2, 2020
valentinsulzer added a commit that referenced this issue Jul 2, 2020
valentinsulzer added a commit that referenced this issue Jul 7, 2020
valentinsulzer added a commit that referenced this issue Jul 21, 2020
valentinsulzer added a commit that referenced this issue Jul 21, 2020
valentinsulzer added a commit that referenced this issue Jul 22, 2020
valentinsulzer added a commit that referenced this issue Jul 23, 2020
valentinsulzer added a commit that referenced this issue Jul 23, 2020
@valentinsulzer
Copy link
Member Author

From discussion this morning with @rtimms , we should look into whether the casadi solver can give information on when it fails. This would allow us to do away with dt_max: first try solving over the whole interval, and if there is a failure then solve up to the point where it fails (and check when/if the event was crossed)

@rtimms
Copy link
Contributor

rtimms commented Jan 18, 2021

Had a little play with this using a solution from here to capture stdout, so seems like something along these lines will work. One thing to be careful of is that you don't fall into the trap of not having enough points in t_eval to do good event detection... I guess if an event is crossed between two times t_{i} and t_{i+1} you could use the "go back and dense solve" method in that interval to get an accurate solution there. As discussed, there is probably a much better way of locating the events.

@valentinsulzer
Copy link
Member Author

If we manage to make this work then we should rethink the default max simulation time in the experiment when doing "until" conditions (currently 1 week, could probably be shorter by default, or based on C-rate)

@valentinsulzer
Copy link
Member Author

Another idea: make the rhs "rhs * switch" where "switch" is a heaviside that turns off when an event is crossed; make the algebraic "algebraic * switch + y * (1- switch)"

@valentinsulzer
Copy link
Member Author

This doesn't work with all events as some have a min or a max, which aren't differentiable (can't calculate jacobian).
Restricting to "voltage" and "current" events for now

valentinsulzer added a commit that referenced this issue Mar 16, 2021
valentinsulzer added a commit that referenced this issue Mar 16, 2021
valentinsulzer added a commit that referenced this issue Mar 16, 2021
valentinsulzer added a commit that referenced this issue Mar 16, 2021
valentinsulzer added a commit that referenced this issue Mar 17, 2021
valentinsulzer added a commit that referenced this issue Mar 17, 2021
valentinsulzer added a commit that referenced this issue Mar 17, 2021
valentinsulzer added a commit that referenced this issue Mar 22, 2021
valentinsulzer added a commit that referenced this issue Mar 22, 2021
@valentinsulzer valentinsulzer mentioned this issue Mar 27, 2021
8 tasks
valentinsulzer added a commit that referenced this issue Mar 27, 2021
valentinsulzer added a commit that referenced this issue Mar 27, 2021
valentinsulzer added a commit that referenced this issue Mar 27, 2021
valentinsulzer added a commit that referenced this issue Mar 27, 2021
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.

2 participants