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

Robustness of move_std #99

Closed
jaimefrio opened this issue Mar 19, 2015 · 6 comments · Fixed by #100
Closed

Robustness of move_std #99

jaimefrio opened this issue Mar 19, 2015 · 6 comments · Fixed by #100

Comments

@jaimefrio
Copy link
Contributor

The algorithm used by move_std is prone to numerical errors. A while back I rewrote the implementation of the equivalent function in Pandas to use what some call Welford's method, a one pass stable algorithm, see here for the relevant PR. I took some timings on the Pandas code, and it resulted in a 25% slowdown.

Would you like to see something like that implemented for bottleneck?

@kwgoodman
Copy link
Collaborator

Sounds like a good idea to me. But I'd like to hear what others think. Anyone?

bn.nanstd uses a two-pass algorithm. If the bottleneck is memory access, which I believe it is, then perhaps the one-pass algorithm you are proposing is faster. If so, assuming comparable stability, then maybe the place to start is with bn.nanstd?

@jaimefrio
Copy link
Contributor Author

If you are not frontally opposed to it I'll put a PR together, and we can discuss its merits after it is up and running.

@kwgoodman
Copy link
Collaborator

That's great. Is this for move_std or nanstd?

@jaimefrio
Copy link
Contributor Author

I think it is only clearly advantageous in move_std, the two pass algorithm is typically faster, and probably even more accurate, for a non-moving window.

@shoyer
Copy link
Member

shoyer commented Mar 20, 2015

Indeed, this seems like a nice improvement 👍.

@jaimefrio
Copy link
Contributor Author

What should go into the PR? Just the move,pyx file in templates, or the generated C files as well?

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.

3 participants