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

Convolution with bitwise_and multiplication for masked images

    XMLWordPrintable

    Details

    • Type: RFC
    • Status: Withdrawn
    • Resolution: Done
    • 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
            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.
            Hide
            tjenness Tim Jenness added a comment -

            This RFC has been open since October 2019. Is anything going to be done about it or should it be withdrawn?

            Show
            tjenness Tim Jenness added a comment - This RFC has been open since October 2019. Is anything going to be done about it or should it be withdrawn?
            Hide
            tjenness Tim Jenness added a comment -

            Does anyone want to suggest a plan for this RFC that has now been open for 21 months. Should the planned end be moved out at least?

            Show
            tjenness Tim Jenness added a comment - Does anyone want to suggest a plan for this RFC that has now been open for 21 months. Should the planned end be moved out at least?
            Hide
            yusra Yusra AlSayyad added a comment -

            My oversimplified reading of this RFC: this RFC was born out of a need for more reasonable convolutions of the mask planes. No disagreement there. The proposed solution of modifying the behavior to use bitwise_and was desired by no one, including the original proposer. Turns out reasonable convolutions of mask planes can be solved by adding functionality, rather than modifying behavior of existing software primitives. Adding functionality does not require an RFC.

            Therefore, we should withdraw this RFC.

            Show
            yusra Yusra AlSayyad added a comment - My oversimplified reading of this RFC: this RFC was born out of a need for more reasonable convolutions of the mask planes. No disagreement there. The proposed solution of modifying the behavior to use bitwise_and was desired by no one, including the original proposer. Turns out reasonable convolutions of mask planes can be solved by adding functionality, rather than modifying behavior of existing software primitives. Adding functionality does not require an RFC. Therefore, we should withdraw this RFC.
            Hide
            yusra Yusra AlSayyad added a comment -

            I haven't heard any disagreements with the recommendation to withdraw.

            Show
            yusra Yusra AlSayyad added a comment - I haven't heard any disagreements with the recommendation to withdraw.

              People

              Assignee:
              gkovacs Gabor Kovacs [X] (Inactive)
              Reporter:
              gkovacs Gabor Kovacs [X] (Inactive)
              Watchers:
              Colin Slater, Eric Bellm, Gabor Kovacs [X] (Inactive), John Parejko, John Swinbank, Robert Lupton, Tim Jenness, Yusra AlSayyad
              Votes:
              0 Vote for this issue
              Watchers:
              8 Start watching this issue

                Dates

                Created:
                Updated:
                Resolved:
                Planned End:

                  Jenkins

                  No builds found.