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

DM-39976: Replace masked image pixels with median values before convolution #271

Merged
merged 2 commits into from Sep 20, 2023

Conversation

isullivan
Copy link
Contributor

Only use with preconvolution and the Score image used for detection for now.

Comment on lines 1038 to 1067


def _interpolateImage(maskedImage, badMaskPlanes, fallbackValue=None):
"""Replace masked image pixels with interpolated values.

Parameters
----------
maskedImage : `lsst.afw.image.MaskedImage`
Image on which to perform interpolation.
badMaskPlanes : `list` of `str`
List of mask planes to interpolate over.
fallbackValue : `float`, optional
Value to set when interpolation fails.
"""
image = maskedImage.image.array
badPixels = (maskedImage.mask.array & maskedImage.mask.getPlaneBitMask(badMaskPlanes)) > 0
image[badPixels] = np.nan
if fallbackValue is None:
fallbackValue = np.nanmedian(image)
# For this initial implementation, skip the interpolation and just fill with
# the median value.
image[badPixels] = fallbackValue
Copy link
Contributor

Choose a reason for hiding this comment

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

This is good.
Fallback value could also be a local quantity? Probably for next change.

Copy link
Contributor

@BrunoSanchez BrunoSanchez 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 good.

I said I would validate with a run and I missed it.
Should I add that?

@isullivan
Copy link
Contributor Author

I think we can discuss new tickets with more detailed analysis and improvements using actual interpolation during sprint planning.

Only use with preconvolution and the Score image used for detection for now.
@BrunoSanchez
Copy link
Contributor

I have reviewed the changes and the notebooks with the results of run comparisons.
This is surprisingly simple and yet yields noticeable lower amount of artifact transients.

@@ -372,6 +372,8 @@ def run(self, template, science, sources, finalizedPsfApCorrCatalog=None,
)
self.log.info("Science PSF FWHM: %f pixels", sciencePsfSize)
self.log.info("Template PSF FWHM: %f pixels", templatePsfSize)
self.metadata.add("sciencePsfSize", sciencePsfSize)
self.metadata.add("templatePsfSize", templatePsfSize)
Copy link
Contributor

Choose a reason for hiding this comment

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

Fine by me, adding metadata is not enough for us to do another ticket.

@isullivan isullivan merged commit aca53a8 into main Sep 20, 2023
2 checks passed
@isullivan isullivan deleted the tickets/DM-39976 branch September 20, 2023 23:56
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