srenevey / ode-solvers

Numerical methods to solve ordinary differential equations in Rust.
BSD 3-Clause "New" or "Revised" License
64 stars 26 forks source link

Solout for other solvers and allocation reduction in Rk4 #12

Closed StefanMathis closed 9 months ago

StefanMathis commented 2 years ago

Hello

this PR does three things:

1) Add an abort condition for the solvers Rk4 and Dop853 via the solout-method (analogous to the existing solution for Dopri5) and added corresponding unit tests for both solvers which check the solout-method.

2) Fix a bug in the solout condition for Dopri5. The newly added unit test in dopri5.rs expects the last time step to be 0.5 (due to the abort condition x >= 0.5 on line 468 (because this is the behaviour I would expect from the solver). However, due to the break condition in fn solution_output, the actual last time step would be 0.4 in the unit test. Hence, I deleted the break condition because from my point of view, it leads to unintuitive behaviour. However, if you think otherwise, I could also revert this change (however, in this case I suggest adding some documentation).

3) Remove allocations from the hot loop of Rk4 by preallocating buffers for k and a new "buffer ", which holds the sum of y and k[x] and is populated with the new function "populate_buffer". I also found that it is not necessary to use a vector of length 12 for k - an array of length 4 does the job just as well, hence why I used one for the buffer.

I'd be happy if you considered merging those changes. All previously existing unit tests are still here and passed with the new changes (especially those in rk4.rs).

EDIT: https://github.com/srenevey/ode-solvers/pull/11 can be rejected if this PR is merged.

thdhondt commented 1 year ago

Thanks a lot for the solout-method for Rk4! This was really helpful.

Any chance this will be merged into the main crate?

Kaikallon commented 1 year ago

Any chance this will ever be merged? I think these changes are great!

dimpdash commented 11 months ago

Really keen to see this merged

DarthB commented 9 months ago

That's a great change, I also run into solout not working!