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

Solving FFT instability issues #7

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

zeeshansayyed
Copy link

This PR addresses #4

The error arises because of a check which in place to make sure that all the x_i values are real. This is done by the function check_xi_are_real() on line 261 of poibin.py. Due to instability in FFT, the imaginary parts don't turn out to be exactly 0. To ensure that these imaginary parts are indeed small, the following check has been placed in code:

xi_values.imag <= np.finfo(float).eps

On my machine, the value of eps turns out to be 2.220446049250313e-16

It turns out, this value is not enough to account for the instability in FFT. Increasing this slightly up to 1e-15 solves this problem.

I feel that making this change is a practical solution to the problem and will still not hurt accuracy.

@tsakim
Copy link
Owner

tsakim commented Aug 3, 2020

Thanks @zeeshansayyed, I'll try it out before merging.

@tsakim tsakim added the bug label Aug 3, 2020
@RoelantStegmann
Copy link

Any news on this? :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants