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

WIP: Modify median filters to handle edges better #38

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

Conversation

jehturner
Copy link
Member

@jehturner jehturner commented May 25, 2018

As per #12, here's a version of PyMedFilt5 that avoids edge residuals, behaving the same as IRAF's median with its default boundary="nearest" and ndimage.median_filter with the equivalent option.

It would be far cleaner to factor this into its own function that each median filter can call, but I notice that you haven't done that elsewhere, presumably for optimization reasons? It would save quite a lot of repetition here though -- what do you think? Also, would you prefer the edge filtering to be optional, since you say you have encountered artifacts associated with filtering at the edges? I'd suggest giving this method a try first, since I don't see obvious problems with it (even if the theory is a bit questionable).

I should perhaps move all the for loops into a single block and avoid re-allocating medarr...

jehturner added 2 commits May 25, 2018 15:58
…in IRAF, extending the window using nearest edge pixel values, rather than just leaving them unfiltered (which led to CR residuals at edges compared with IRAF).
else if (j+k >= ny) nxk = nx * (ny-j-1);
else nxk = nx * k;
for (l = -2; l < 3; l++) {
ipl = i+l;
Copy link
Member Author

Choose a reason for hiding this comment

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

Sorry, EMACS put some tabs in. I'll get rid of them in the next version.

@jehturner
Copy link
Member Author

As a quick performance check, timeit shows that median filtering a 4176x512 pixel image 100 times takes ~12s on my 6-core workstation with both the old and new versions of PyMedFilt5 (for some reason the exact time varies by about 0.5s, but the difference is within the noise). This isn't really a surprise, since the behaviour only differs for 4 rows/columns out of 4176 or 512.

jehturner added 2 commits May 28, 2018 11:42
…d median).

This seems about 8% slower than the 5x5-specific version, probably because the compiler can't optimize the loops (as per the existing comment), but the more general NxN version may be useful anyway.
@jehturner
Copy link
Member Author

Here's a version that reimplements the 5x5 median filter using an NxN median filter. From a quick check, it seems to be about 8% slower, presumably because the compiler can't optimize the inner loops as per your comment in the existing code -- but maybe it's useful to have an NxN filter anyway and I'll push it so you can have a look. Would you rather keep the triplicated cut-and-paste versions for performance? It's probably going to be quite fiddly to abstract them without some run-time penalty (eg. using the pre-processor or generated code?).

@coveralls
Copy link

coveralls commented May 29, 2018

Coverage Status

Coverage remained the same at 100.0% when pulling 4876fc2 on jehturner:issue_12 into 3ed58dd on astropy:master.

@jehturner
Copy link
Member Author

jehturner commented Jun 15, 2018

This last update made similar changes to clean_medmask as for median filtering, so the edges aren't ignored there either. Since cleaning is part of the iterative process, this also slightly improves the detection of CRs at the edges and gives results close to lacos_spec in IRAF (still not quite identical).

Edge pixels are also being ignored in the Laplace and dilation/growth filtering, but those don't seem to make such a difference to the end result when changed. I think the current Laplace filter is arguably incorrect at the edges of the array, because the central value of 4 should be adjusted to 3 or 2 to account for the number of pixels being subtracted from it, otherwise the gradient measurement is dominated by superimposed signal in those rows/columns. As I say, however, it's the median filtering and cleaning that seem to make most of the difference to CR residuals at edges.

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