# Update large masks for BF convolution issues

XMLWordPrintable

## Details

• Type: Improvement
• Status: Done
• Resolution: Done
• Fix Version/s: None
• Component/s:
• Labels:
None
• Story Points:
6
• Team:
Data Release Production

## Description

Brighter-fatter correction includes a convolution, which means that pixels located near a masked region may be modified incorrectly due to missing data. Grow large masked regions after BF correction to account for this.

This modification will repurpose the IsrTask.maskEdges() method, which was originally designed to fix this exact issue around detector edges.

## Activity

Hide
Yusra AlSayyad added a comment - - edited

Since this is a major behavior change, let's make sure this is tested thoroughly and we'e thought of all the ways we can do this. Convolutions are expensive. This seems to add 10 extra seconds to brighter-fatter correction which AP cannot afford.

 processCcd.isr INFO: Finished brighter fatter correction in 9 iterations. 28.955341339111328 processCcd.isr WARN: Ensuring image edges are masked as SUSPECT to the brighter-fatter kernel size. 28.957067728042603 processCcd.isr WARN: Ensuring image is masked as BAD near large previously-BAD masks. 38.82804036140442 

Also yikes. We can't really afford 28 seconds for brighter fatter correction either.

Also INFO is probably more appropriate than WARN.
Do we need new config parameters?

Show
Yusra AlSayyad added a comment - - edited Since this is a major behavior change, let's make sure this is tested thoroughly and we'e thought of all the ways we can do this. Convolutions are expensive. This seems to add 10 extra seconds to brighter-fatter correction which AP cannot afford. processCcd.isr INFO: Finished brighter fatter correction in 9 iterations. 28.955341339111328 processCcd.isr WARN: Ensuring image edges are masked as SUSPECT to the brighter-fatter kernel size. 28.957067728042603 processCcd.isr WARN: Ensuring image is masked as BAD near large previously-BAD masks. 38.82804036140442 Also yikes. We can't really afford 28 seconds for brighter fatter correction either. Also INFO is probably more appropriate than WARN. Do we need new config parameters?
Hide

For processCcd.py /datasets/hsc/repo --rerun private/yusra/test_defects_pr --calib /home/yusra/lsst_devel/LSST/DMS/obs_subaru_master/hsc/CALIB --id visit=130334 ccd=33 --no-versions where /home/yusra/lsst_devel/LSST/DMS/obs_subaru_master/hsc/CALIB contains my outrageously large test defect mask for CCD33:

Before/After:
Before/After:

Show
Yusra AlSayyad added a comment - For processCcd.py /datasets/hsc/repo --rerun private/yusra/test_defects_pr --calib /home/yusra/lsst_devel/LSST/DMS/obs_subaru_master/hsc/CALIB --id visit=130334 ccd=33 --no-versions where /home/yusra/lsst_devel/LSST/DMS/obs_subaru_master/hsc/CALIB contains my outrageously large test defect mask for CCD33: Before/After: Before/After:
Hide
Paul Price added a comment -

I don't think we want to grow the mask by the full width of the convolution kernel, but by something like the FWHM of the kernel.

Show
Paul Price added a comment - I don't think we want to grow the mask by the full width of the convolution kernel, but by something like the FWHM of the kernel.
Hide
Paul Price added a comment -

And the grow operation could be done using SpanSet rather than a convolution.

Show
Paul Price added a comment - And the grow operation could be done using SpanSet rather than a convolution.
Hide

Indeed.

However, the convolution in this function wasn't being used for growing, but rather distinguishing between pixels on the edge of a large defect region that were destroyed by the BF conv vs. the SAT pixels around an e.g. bleed trail which don't take too many pixels out of play for the BF conv to cause damage.

The images above are to show that the parameters (kernel size, threshold) are not giving it the intended behavior.

Show
Yusra AlSayyad added a comment - Indeed. However, the convolution in this function wasn't being used for growing, but rather distinguishing between pixels on the edge of a large defect region that were destroyed by the BF conv vs. the SAT pixels around an e.g. bleed trail which don't take too many pixels out of play for the BF conv to cause damage. The images above are to show that the parameters (kernel size, threshold) are not giving it the intended behavior.
Hide
Christopher Waters added a comment -

I'm having some trouble reproducing the saturation spike images, but I have switched to growing the masks by a fixed amount to ensure that this isn't slowing down ISR processing (with the benefit that the saturation spike now has a more reasonable looking mask). The results appear to be nearly identical, with the added bonus that the configuration parameter is easier to understand.

I'll squash the commits after confirming there are no other cases to check.

Show
Christopher Waters added a comment - I'm having some trouble reproducing the saturation spike images, but I have switched to growing the masks by a fixed amount to ensure that this isn't slowing down ISR processing (with the benefit that the saturation spike now has a more reasonable looking mask). The results appear to be nearly identical, with the added bonus that the configuration parameter is easier to understand. I'll squash the commits after confirming there are no other cases to check.
Hide

The images posted above are the before and after the top hat function was called. e.g.

    ccdExposure.writeFits('/home/yusra/public_html/download/beforeNewMaskGrowTophat.fits')  self.maskGrowTophat(ccdExposure, boxRadius=numpy.max(bfKernel.shape) // 2,  boxThreshold=0.25,  maskPlane=self.config.maskListToInterpolate,  maskLabel="BAD")  self.log.warn("Ensuring image is masked as BAD near large previously-BAD masks.")  ccdExposure.writeFits('/home/yusra/public_html/download/afterNewMaskGrowTophat.fits')  self.debugView(ccdExposure, "doBrighterFatter")

Going to pull your latest changes and take a look at their behavior now.

Show
Hide

Behavior now grows each of the masks in config.maskListToInterpolate after brighter-fatter correction. This makes sense, because these masks are the only ones brighter-fatter is aware of.

Looked at a bunch Friday night and concluded that config.brighterFatterMaskGrowSize = 1 is sufficient.

Final Calexps (BEFORE THIS TICKET) vs (AFTER THIS TICKET) 1-pixel:

Intermediate right BEFORE MASKING and AFTER MASKING:

Show
Yusra AlSayyad added a comment - Behavior now grows each of the masks in config.maskListToInterpolate after brighter-fatter correction. This makes sense, because these masks are the only ones brighter-fatter is aware of. Looked at a bunch Friday night and concluded that config.brighterFatterMaskGrowSize = 1 is sufficient. Final Calexps (BEFORE THIS TICKET) vs (AFTER THIS TICKET) 1-pixel:   Intermediate right BEFORE MASKING and AFTER MASKING:

## People

• Assignee:
Christopher Waters
Reporter:
Christopher Waters
Reviewers:
Watchers:
Christopher Waters, Paul Price, Yusra AlSayyad