Skip to content

Improved Gauss kernel generation#2844

Open
RunDevelopment wants to merge 1 commit into
image-rs:mainfrom
RunDevelopment:impr-kernel-gen
Open

Improved Gauss kernel generation#2844
RunDevelopment wants to merge 1 commit into
image-rs:mainfrom
RunDevelopment:impr-kernel-gen

Conversation

@RunDevelopment
Copy link
Copy Markdown
Member

Changes:

  • Added debug assertions to check assumptions in debug.
  • Define a minimum sigma of 0.01. This prevents division by zero.
  • Removed the scaling by $1/\sqrt{2\pi\sigma^2}$. Since kernel weights are normalized, this scaling is unnecessary.
  • Remove use of f32::powf and cache more divisions. The performance impact will likely be negligible, but I'd rather remove functions of unknown complexity.

Taken together, get_gaussian_kernel_1d now generates correct kernels for all $\sigma\ge 0$.

Since I only changed the behavior for edge cases (very small sigma), the behavior of our Gaussian blur remains unchanged.

Copy link
Copy Markdown
Contributor

@mstoeckl mstoeckl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks like a strict improvement over the preceding code; I do not see any new floating point issues.

(It doesn't fix all of them -- the calculation of sum_norm is of course increasingly affected by floating point error when sigma and width are huge. But fixing this would make the function much slower or much more complicated, and has no benefit when the image filtering code doesn't do the same. And huge sigma and width are incredibly slow to evaluate anyway.)

@RunDevelopment
Copy link
Copy Markdown
Member Author

Thanks for the review!

the calculation of sum_norm is of course increasingly affected by floating point error when sigma and width are huge.

Yes, but I don't think we need to worry about the precision of sum_norm. For a sigma of 10k, I see a relative error of around 0.0003%. (That said, we can lower this error by exploiting that the kernel is symmetric. This (approximately) halves the relative error.)

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.

2 participants