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

Min Step Size Bug #20

Closed
jrenaud90 opened this issue Mar 23, 2023 · 1 comment · Fixed by #23
Closed

Min Step Size Bug #20

jrenaud90 opened this issue Mar 23, 2023 · 1 comment · Fixed by #23
Labels
bug Something isn't working cython Issue related to cython-based code nbrk_ode An issue with the Numba solver

Comments

@jrenaud90
Copy link
Owner

jrenaud90 commented Mar 23, 2023

During the original implementation of CyRK I misunderstood how the minimum step size check was supposed to work. Scipy defines the minimum step size based on the actual value of time,

import numpy as np
min_step = 10 * np.abs(np.nextafter(t, direction * np.inf) - t)

whereas CyRK originally had a constant step size based off of the floating point error,
defines the minimum step size based on the actual value of time,

import numpy as np
min_step = 10 * np.finfo(np.float64).eps

This latter approach works for when t is close to zero, but is not accurate when t is far from zero since the number of floats between numbers decreases at higher values.

Since min_step is used to set the step_size from becoming too small during integration - having an incorrect value could lead to incorrect or unexpected behavior.

This bug affects both the cyrk_ode and nbrk_ode.

image

@jrenaud90 jrenaud90 changed the title "next after" potential bug Min Step Size Bug Mar 28, 2023
@jrenaud90 jrenaud90 added bug Something isn't working nbrk_ode An issue with the Numba solver cython Issue related to cython-based code labels Mar 28, 2023
@jrenaud90 jrenaud90 linked a pull request Mar 29, 2023 that will close this issue
jrenaud90 added a commit that referenced this issue Mar 29, 2023
* Fixed min step bug in `nbrk_ode` and `cyrk_ode` (Fixes Issue #20)
* Minor performance improvements for `cyrk_ode`
* Removed fastmath for `nbrk_ode` (Created Issue #24)
@jrenaud90 jrenaud90 mentioned this issue Mar 29, 2023
jrenaud90 added a commit that referenced this issue Mar 29, 2023
Major Changes:
- Fix for Issue #20
- Fix for Issue #3
- Minor performance improvements
- Additional tests
- Refactors and cleanups for `cyrk_ode`

---------

Co-authored-by: David Meyer <dihm@users.noreply.github.com>
@jrenaud90
Copy link
Owner Author

Closed by PR #25 and v0.5.0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working cython Issue related to cython-based code nbrk_ode An issue with the Numba solver
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant