Uploaded image for project: 'Request For Comments'
  1. Request For Comments
  2. RFC-638

Convolution with bitwise_and multiplication for masked images

    Details

    • Type: RFC
    • Status: Proposed
    • Resolution: Unresolved
    • Component/s: DM
    • Labels:
      None

      Description

      Triggered by DM-16544, this is a proposal for a

      • MaskedImagePixel multiplication operation (perhaps function, not operator) that performs bitwise_and in the mask plane
      • a convolution function that uses the bitwise_and multiplication and bitwise_or addition

      At the moment all (+,-,*,/) pixel operators perform bitwise_or as the mask operation. Actually we do not have any operations with bitwise_and. This causes the mask flags to spread into relatively large image areas during convolution, even if the convolution kernel has a small effective area (pixels where it is not zero) within its container. This variant of the convolution function would enable the caller to define which part of the convolution kernel is significant from the point of accumulating mask flags.

      Additional clarification 2019-10-28:

      At the moment image convolution, as far as I understand, is implemented to perform the calculations directly on generic (template) pixel types include/lsst/afw/math/ConvolveImage.h which include the MaskedImagePixel. I.e. the convolution steps of masked images are directly performed on a per pixel basis on the pixel, variance and mask planes, with their respective * (*=) and + (+=) operators. The mask plane operation is always bitwise_or.  While there is a if (kval !=0) filter in the convolution implementation to ignore zero kernel values, this would skip integer or float exact zeroes only, but in general e.g. ip_diffim AL basis kernels are nowhere exactly zeroes. Thus a convolved image (e.g. the psf matched image that then goes into the difference image) accumulates all the flags from a kernelsize x kernelsize area around each pixel.

        Attachments

          Issue Links

            Activity

            Hide
            tjenness Tim Jenness added a comment -

            Gabor Kovacs what is the status of this RFC?

            Show
            tjenness Tim Jenness added a comment - Gabor Kovacs what is the status of this RFC?
            Hide
            swinbank John Swinbank added a comment -

            Gabor is distracted by Gen3 migration at the moment. I suggest we punt the due date of this to mid-December, then see how things look.

            Show
            swinbank John Swinbank added a comment - Gabor is distracted by Gen3 migration at the moment. I suggest we punt the due date of this to mid-December, then see how things look.
            Hide
            gkovacs Gabor Kovacs added a comment - - edited

            Please see demonstration notebook attached.

            As discussion on this RFC stalled a long time ago, I'd re-phrase the proposal as changing afwMath.convolve to support:

            afwMath.convolve(result.mask, image.mask, kernel, convCntrl)

            where result.mask, image.mask are the mask planes (image.MaskX) of MaskedImage, kernel, convCtrl are the same as currently are. The behavior should be the same as in the MaskedImage case:

            afwMath.convolve(result2, image, kernel, convCntrl)  # result.mask == result2.mask

            This would make it straightforward to perform the convolution with 2 calls, with a tailored kernel on the mask plane (and 1 wasted operation on the mask plane in the 1st call):

             

            afwMath.convolve(im1conv, im1ex, mKernel, convCntrl)  # Currently supported and works on .image .variance and .mask
            afwMath.convolve(im1conv.mask, im1ex.mask, ExactZeroOrSmallerSizeKernel, convCntrl)  # Currently not supported


            or/and:

            adding of propagationAndMask as an optional argument for MaskedImage convolution:

            afwMath.convolve(result, image, kernel, convCtrl, propagationAndMask)

            where

            R = 0
            for i,k:     
             
            #  optionally keeping the current behavior and doing only for the non-zero kernel pixels:    
             
            #  if kernel[i,k] != 0 :
             
              R |= image.mask[x-i, y-k] & propagationAndMask[i, k] 
            result.mask[x,y] = R
            

             and if propagationAndMask is missing it defaults to 0xFFFFFFFF (whatever number of bits we need), of course.

            Show
            gkovacs Gabor Kovacs added a comment - - edited Please see demonstration notebook attached. As discussion on this RFC stalled a long time ago, I'd re-phrase the proposal as changing afwMath.convolve to support: afwMath.convolve(result.mask, image.mask, kernel, convCntrl) where result.mask , image.mask are the mask planes ( image.MaskX ) of MaskedImage , kernel , convCtrl are the same as currently are. The behavior should be the same as in the MaskedImage case: afwMath.convolve(result2, image, kernel, convCntrl)  # result.mask == result2.mask This would make it straightforward to perform the convolution with 2 calls, with a tailored kernel on the mask plane (and 1 wasted operation on the mask plane in the 1st call):   afwMath.convolve(im1conv, im1ex, mKernel, convCntrl) # Currently supported and works on .image .variance and .mask afwMath.convolve(im1conv.mask, im1ex.mask, ExactZeroOrSmallerSizeKernel, convCntrl) # Currently not supported or/and: adding of propagationAndMask as an optional argument for MaskedImage convolution: afwMath.convolve(result, image, kernel, convCtrl, propagationAndMask) where R = 0 for i,k:   # optionally keeping the current behavior and doing only for the non-zero kernel pixels:   # if kernel[i,k] != 0 :   R |= image.mask[x-i, y-k] & propagationAndMask[i, k] result.mask[x,y] = R  and if propagationAndMask is missing it defaults to 0xFFFFFFFF (whatever number of bits we need), of course.
            Hide
            gkovacs Gabor Kovacs added a comment - - edited

            Second thought, considering what to do with the mask in zogy, I'd prefer the 1st option in the form:

            Keep the current functions, as they are now, i.e.:

            afwMath.convolve(result, mimage, kernel, convCntrl)
            afwMath.convolve(result.image, mimage.image, kernel, convCntrl)

            would work on MaskedImage and Image objects.

            Adding

            afwMath.convolveVariancePlane(result.variance, mimage.variance, kernel, convCntrl)

            would do the variance plane part of the MaskedImage operation (i.e. convolution by kernel**2).

            Adding

            afwMath.convolveMaskPlane(result.mask, mimage.mask, kernel, convCntrl)

            would do the standalone mask plane operation where kernel is integer type and functions as an And mask, i.e.:

            R = 0 
            for i,k:
                R |= image.mask[x-i, y-k] & kernel[i, k]  
            result.mask[x,y] = R  

            If mask kernel contains only 0 and 0xFFFFFFFF pixels, this result will be the same as the mask plane operation on MaskedImages above corresponding to kernel==0 and kernel != 0 pixels respectively.

            Show
            gkovacs Gabor Kovacs added a comment - - edited Second thought, considering what to do with the mask in zogy, I'd prefer the 1st option in the form: Keep the current functions, as they are now, i.e.: afwMath.convolve(result, mimage, kernel, convCntrl) afwMath.convolve(result.image, mimage.image, kernel, convCntrl) would work on MaskedImage and Image objects. Adding afwMath.convolveVariancePlane(result.variance, mimage.variance, kernel, convCntrl) would do the variance plane part of the MaskedImage operation (i.e. convolution by kernel**2). Adding afwMath.convolveMaskPlane(result.mask, mimage.mask, kernel, convCntrl) would do the standalone mask plane operation where kernel is integer type and functions as an And mask, i.e.: R = 0 for i,k: R |= image.mask[x-i, y-k] & kernel[i, k] result.mask[x,y] = R If mask kernel contains only 0 and 0xFFFFFFFF pixels, this result will be the same as the mask plane operation on MaskedImages above corresponding to kernel==0 and kernel != 0 pixels respectively.
            Hide
            rhl Robert Lupton added a comment -

            I'm sorry, but I can't follow this discussion. Can you write a description of what you are trying to achieve at this point rather than a proposed implementation, maybe via a new RFC? You seem to be mixing up "only propagate masks from pixels that have a large kernel coefficient" and "only propagate some bits".

            For example, comments like "considering what to do with the mask in zogy" doesn't help me understand what you are trying to achieve.

            Show
            rhl Robert Lupton added a comment - I'm sorry, but I can't follow this discussion. Can you write a description of what you are trying to achieve at this point rather than a proposed implementation, maybe via a new RFC? You seem to be mixing up "only propagate masks from pixels that have a large kernel coefficient" and "only propagate some bits". For example, comments like "considering what to do with the mask in zogy" doesn't help me understand what you are trying to achieve.

              People

              • Assignee:
                gkovacs Gabor Kovacs
                Reporter:
                gkovacs Gabor Kovacs
                Watchers:
                Colin Slater, Eric Bellm, Gabor Kovacs, John Parejko, John Swinbank, Robert Lupton, Tim Jenness
              • Votes:
                0 Vote for this issue
                Watchers:
                7 Start watching this issue

                Dates

                • Created:
                  Updated:
                  Planned End:

                  Summary Panel