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

2D Tophat (Disk) Kernel #165

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Conversation

mileslucas
Copy link

This adds a 2D "tophat" kernel. This is geometrically just a disk. I'm not an images guy, per se, so if there is a more apt name than tophat, please inform me!

The two functions added to Kernel are

  • tophat(r) - default size is 2r rounded up to the nearest odd integer. The cells whose distance from the center is less than or equal to r will be constant and all other will be zero. The constant will be such that the sum is 1.
  • tophat(r, ls) - the same as above, but allos user-input size ls. This will handle even and odd based shapes, and even mixes of both (eg (6, 7). Again, the pixels whose distance from the center is less than or equal to r will be a normalized constant.

I've added a docstring and imported correctly under ImageFiltering.jl. I've added it to the Kernel module docstring, too. Unfortunately I'm not having a great time parsing the test suite, so I don't have any tests. Please let me know where I can test this.

@mileslucas
Copy link
Author

Bumping this

@jw3126
Copy link
Collaborator

jw3126 commented May 12, 2020

I think this can be tested in a @testset "tophat" in test/2d.jl. Is this tophat function coming from matlab or somehow standard? There are a lot of variantions on how to turn a circle into pixels. E.g. constant value strictly inside, value proportional to circle area intersected with pixel etc.

@mileslucas
Copy link
Author

This implementation is based off the skimage.draw.disk implementation which is used in the python library which inspired this PR.

Copy link
Collaborator

@jw3126 jw3126 left a comment

Choose a reason for hiding this comment

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

Thanks for the PR, see my above remarks and some tests would be good.

@@ -416,6 +417,34 @@ moffat(α::Real, β::Real) = moffat(α, β, ceil(Int, (α*2*sqrt
sum(x.^2)
end

"""
tophat(r) -> k
tophat(r, ls) -> k
Copy link
Collaborator

Choose a reason for hiding this comment

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

Rename ls to size and explain it?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Also some doctests would be nice.

kern[idx] = amplitude
end
end
return kern ./ sum(kern) # renormalize
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why do you compute amplitude above, if you normalize it away here anyway?

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.

None yet

2 participants