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

Enhance numerical stability of Savitzky-Golay coefficient computation. #1

Open
wants to merge 9 commits into
base: main
Choose a base branch
from

Conversation

MothNik
Copy link

@MothNik MothNik commented Jun 20, 2024

This PR adds a numerically stabilised version of the Savitzky Golay filter computation as described for the Scipy Issue #20825 in this comment.

Added a numerically stabilised version of the Savitzky Golay filter computation as described in
Scipy Issue #20825
scipy/scipy#20825 (comment)
@MothNik
Copy link
Author

MothNik commented Jun 20, 2024

The adaption mentioned here seems possible without loss of accuracy.

…y avoiding the explicit computation of the pseudoinverse
MothNik added 7 commits June 23, 2024 11:35
…tion for the super stabilised Savitzky Golay coefficients

- in this context, the derivative pre-factor was incorporated into the correction factors as a single rather than multiple multiplications
…on coefficients of the super stabilised Savitzky Golay coefficients. It does not provide any benefit and `math.factorial` is more direct. Yet, it's a micro-optimization.
…this could ever be encountered in this context
…/ 2`` in super stabilised Savitzky Golay coefficient computation by using a simplified solution in this case

- made relative error test in main truly insensitive to ZeroDivision (`inf` was never used in case of zero division because Python does not evaluate lazily)
- added a more extensive test for the coefficients against the correct solution to main to avoid thing like the first bullet point happening
…Golay coefficients safe against floating point errors in the zero-check
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 this pull request may close these issues.

1 participant