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

Add bin splitting to filters.rank.percentile_mean #7111

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

lagru
Copy link
Member

@lagru lagru commented Aug 28, 2023

Description

Fixes #7096.

The algorithm works by summing bins of a histogram of the surrounding neighborhood together while skipping bins outside the defined percentile. However, the previous implementation would only deal with complete bins leading to unexpected behavior when according to the given percentile, a part of a bin should factor into the mean. The new implementation fixes this problem, while increasing performance such that it is on par with rank.mean for most tested scenarios.

Checklist

Release note

Summarize the introduced changes in the code block below in one or a few sentences. The
summary will be included in the next release notes automatically:

...

The algorithm works by summing bins of a histogram of the surrounding
neighborhood together while skipping bins outside the defined
percentile. However, the previous implementation would only deal with
complete bins leading to unexpected behavior when according to the
given percentile, a part of a bin should factor into the mean. The
new implementation fixes this problem, while increasing performance such
that it is on par with rank.mean for most tested scenarios.
@lagru lagru added the 🩹 type: Bug fix Fixes unexpected or incorrect behavior label Aug 28, 2023
Previously, if a pixel was directly on the pixel border its value wasn't
taken into account for the average. The new test, should ensure the
behavior. Removing the extra `lower - 1` or `upper + 1` turns the test
red.
@lagru
Copy link
Member Author

lagru commented Aug 29, 2023

It's tricky to figure out the correct behavior for the edges of the inclusive percentile interval [p0, p1]. I think it's still not correct.

@lagru lagru marked this pull request as draft August 29, 2023 16:33
Copy link

Hello scikit-image core devs! There hasn't been any activity on this PR for more than 180 days. I have marked it as "dormant" to make it easy to find.
To our contributors, thank you for your contribution and apologies if this contribution fell through the cracks! Hopefully this ping will help bring some fresh attention to the review. If you need help, you can always reach out on our forum
If you think that this PR is no longer relevant, you may close it, or we may do it at some point (either way, it will be done manually). If you think the PR is valuable but you no longer have the bandwidth to update it, please let us know, so that someone can take it over. 🙏

@github-actions github-actions bot added the 😴 Dormant no recent activity label Feb 26, 2024
@github-actions github-actions bot removed the 😴 Dormant no recent activity label Apr 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🩹 type: Bug fix Fixes unexpected or incorrect behavior
Projects
None yet
Development

Successfully merging this pull request may close these issues.

filters.rank.mean_percentile gives unexpected zeros
1 participant