-
Notifications
You must be signed in to change notification settings - Fork 8
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-45361: Improve maskStreaks line fitting and mask plane behavior #363
Conversation
@@ -22,6 +22,8 @@ | |||
import numpy as np | |||
|
|||
import lsst.afw.detection as afwDetection | |||
import lsst.afw.image as afwImage | |||
import lsst.afw.math as afwMath # FOR TESTING BINNING |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we still need the clarifying comment since afwMath is necessary for making the binned image?
self.config.streakBinFactor) | ||
binnedExposure = afwImage.ExposureF(binnedMaskedImage.getBBox()) | ||
binnedExposure.setMaskedImage(binnedMaskedImage) | ||
binnedExposure.setPsf(difference.psf) # exposure must have a PSF |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So I have been informed that there is a possibility that you can reach this point in the pipeline and an exposure might not have a PSF. How does that get handled in this situation? Does this automatically fail out gracefully or do we need to add some error handling just in case.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for thinking of this — indeed this was a past problem, so I verified that the _prepareInputs
call in the run
method of detectAndMeasure will raise well before we get to streak masking if the PSF is un-compute-shape-able.
self.assertTrue(np.all(streakMaskSet[streak_check])) | ||
# Check that the entire image was not masked STREAK | ||
self.assertFalse(np.all(streakMaskSet)) | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If error handling is needed in the main code, then you should include a null test where things go wrong so you can check that the error is properly caught.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not relevant since we can never land here without a valid PSF
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Everything looks good but Colin informed me that there is a chance that you can reach ip_diffim with no psf, and I wasn't 100% sure how the code would fail out and if it would be graceful. If not, it might require some error handling for the weird edge case.
abf95ee
to
2f3b1b4
Compare
This PR adjusts how maskStreaksTask is implemented in difference imaging. It uses a mask plane binning strategy to more readily detect fainter streaks, thereby "connecting the dots" in the DETECTED mask plane before looking for linear features.