Uploaded image for project: 'Data Management'
  1. Data Management
  2. DM-23083

Update large masks for BF convolution issues

    Details

    • Type: Improvement
    • Status: Done
    • Resolution: Done
    • Fix Version/s: None
    • Component/s: ip_isr
    • Labels:
      None

      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.

        Attachments

          Issue Links

            Activity

            Hide
            yusra 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 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
            yusra 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:

            Show
            yusra 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
            price 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
            price 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
            price Paul Price added a comment -

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

            Show
            price Paul Price added a comment - And the grow operation could be done using SpanSet rather than a convolution.
            Hide
            yusra 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.

            Show
            yusra 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
            czw 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
            czw 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
            yusra Yusra AlSayyad added a comment -

            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
            yusra Yusra AlSayyad added a comment - 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.
            Hide
            yusra 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:

            Show
            yusra 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:
                czw Christopher Waters
                Reporter:
                czw Christopher Waters
                Reviewers:
                Yusra AlSayyad
                Watchers:
                Christopher Waters, Paul Price, Yusra AlSayyad
              • Votes:
                0 Vote for this issue
                Watchers:
                3 Start watching this issue

                Dates

                • Created:
                  Updated:
                  Resolved:

                  Summary Panel