Skip to content

Gaussian smoothing is wrong #102

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

Open
ad8e opened this issue Feb 24, 2025 · 1 comment
Open

Gaussian smoothing is wrong #102

ad8e opened this issue Feb 24, 2025 · 1 comment

Comments

@ad8e
Copy link

ad8e commented Feb 24, 2025

It needs to be a convolution with a Gaussian kernel.
The source repository https://github.com/petoem/taira doesn't do it correctly because it skips the first and last points.
Even if the source repository were correct, you'd have to scale up the kernel as the width (sigma) increased. A decent threshold is 3 * sigma. Currently, it's fixed at kernel size 2*2+1 = 5:

return arr.length > 5 ? smoothing.smoothen(arr, smoothing.ALGORITHMS.GAUSSIAN, 2, Math.round(weight / 100 * 5), false) : arr;

This small kernel causes a big difference at Gaussian smoothing 100, which should be almost flat. But there's still a lot of local variation:

Image

That's because a big Gaussian width with a small kernel size is just a small rectangular window.

It is nontrivial to do performant Gaussian smoothing; a FFT is needed for larger kernels. You can try to find a library that does this; the term is "windowed convolution". If you try to use Taira for this, it will be very slow (and still wrong). One library I found is https://github.com/mljs/convolution, but I have never used it.

With Gaussian smoothing, you should set sigma = "Smoothing Value", rather than what it attempts now, which is Math.round(weight / 100 * 5).

EDIT: actually, since your points can be spaced unevenly, your task may be even harder. The conventional windowed convolution algorithms only work with evenly spaced points. But your existing solution has the same problem, so it won't be a downgrade.

@nirallegro
Copy link
Contributor

@ad8e Thanks for the input. We'll look into it.

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

No branches or pull requests

2 participants