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

Investigate mask propagation through ip_diffim

    Details

      Description

      As first identified in DM-13081, the template mask planes seem to propagate into unreasonably large masks in the difference image. This ticket is to investigate the source of this behavior and fix it, if appropriate. It may be useful to consult with Gabor Kovacs.

        Attachments

          Issue Links

            Activity

            Hide
            swinbank John Swinbank added a comment -

            We (Meredith, Eric, John) discussed this on 2019-01-29 and thought it probably makes sense to assign this to Gabor, although we don't think it should be his immediate priority.

            Show
            swinbank John Swinbank added a comment - We (Meredith, Eric, John) discussed this on 2019-01-29 and thought it probably makes sense to assign this to Gabor, although we don't think it should be his immediate priority.
            Hide
            gkovacs Gabor Kovacs added a comment -

            Ok, I will do debugger runs anyway, so I'll try to identify the relevant codes modifying the masks.

            Show
            gkovacs Gabor Kovacs added a comment - Ok, I will do debugger runs anyway, so I'll try to identify the relevant codes modifying the masks.
            Hide
            swinbank John Swinbank added a comment -

            Gabor Kovacs, Eric Bellm — now that we have a better understanding of ip_diffim in general, do we still regard this ticket as an issue?

            Show
            swinbank John Swinbank added a comment - Gabor Kovacs , Eric Bellm — now that we have a better understanding of ip_diffim in general, do we still regard this ticket as an issue?
            Hide
            swinbank John Swinbank added a comment -

            (I'm assuming that means no).

            Show
            swinbank John Swinbank added a comment - (I'm assuming that means no).
            Hide
            ebellm Eric Bellm added a comment -

            Gabor Kovacs reports that this behavior is due to the mask planes being OR'd during kernel convolutions, even for pixels where the kernel weights are 0.

            Show
            ebellm Eric Bellm added a comment - Gabor Kovacs reports that this behavior is due to the mask planes being OR'd during kernel convolutions, even for pixels where the kernel weights are 0.
            Hide
            gkovacs Gabor Kovacs added a comment -

            The relevant declaration in include/lsst/afw/image/Pixel.h:73

            Pixel.h

            template <typename _ImagePixelT, typename _MaskPixelT, typename _VariancePixelT = double>
            class SinglePixel : public detail::MaskedImagePixel_tag {
            public:
            template <typename, typename, typename>
            friend class Pixel;
            template <typename T>
            friend class PixelTypeTraits;

            typedef _ImagePixelT ImagePixelT;
            typedef _MaskPixelT MaskPixelT;
            typedef _VariancePixelT VariancePixelT;
            ...

            convolution multiplies and adds SinglePixel instances include/lsst/afw/math/ConvolveImage.h:274

            ConvolveImage.h

            inline typename OutImageT::SinglePixel convolveAtAPoint
            ...
            typename OutImageT::SinglePixel outValue = 0;
            ...
            outValue += *inImageLocator * kVal;

            include/lsst/afw/image/Pixel.h:604

            Pixel.h

            template <typename ExprT1, typename ExprT2>
            BinaryExpr<ExprT1, ExprT2, std::multiplies<typename exprTraits<ExprT1>::ImagePixelT>,
            bitwise_or<typename exprTraits<ExprT1>::MaskPixelT>,
            variance_multiplies<typename exprTraits<ExprT1>::VariancePixelT> >
            operator*(ExprT1 e1, ExprT2 e2) {
            return BinaryExpr<ExprT1, ExprT2, std::multiplies<typename exprTraits<ExprT1>::ImagePixelT>,
            bitwise_or<typename exprTraits<ExprT1>::MaskPixelT>,
            variance_multiplies<typename exprTraits<ExprT1>::VariancePixelT> >(e1, e2);
            }

            For all (+,-,*,/) pixel operations, the mask operation is bitwise_or. Actually we do not have any operations with bitwise_and.

            Proposal:
            I'd propose a variant of the convolution operation that performs bitwise_and at the image and kernel pixel multiplication step. So that the kernel as a MaskedImage could carry a region "AND-mask" which would be bitwise_or-ed around a result pixel by the addition step. In this way, regions depending on the kernel value could be defined that determine the mask value of the output pixel.

            Show
            gkovacs Gabor Kovacs added a comment - The relevant declaration in include/lsst/afw/image/Pixel.h:73 Pixel.h template <typename _ImagePixelT, typename _MaskPixelT, typename _VariancePixelT = double> class SinglePixel : public detail::MaskedImagePixel_tag { public: template <typename, typename, typename> friend class Pixel; template <typename T> friend class PixelTypeTraits; typedef _ImagePixelT ImagePixelT; typedef _MaskPixelT MaskPixelT; typedef _VariancePixelT VariancePixelT; ... convolution multiplies and adds SinglePixel instances include/lsst/afw/math/ConvolveImage.h:274 ConvolveImage.h inline typename OutImageT::SinglePixel convolveAtAPoint ... typename OutImageT::SinglePixel outValue = 0; ... outValue += *inImageLocator * kVal; include/lsst/afw/image/Pixel.h:604 Pixel.h template <typename ExprT1, typename ExprT2> BinaryExpr<ExprT1, ExprT2, std::multiplies<typename exprTraits<ExprT1>::ImagePixelT>, bitwise_or<typename exprTraits<ExprT1>::MaskPixelT>, variance_multiplies<typename exprTraits<ExprT1>::VariancePixelT> > operator*(ExprT1 e1, ExprT2 e2) { return BinaryExpr<ExprT1, ExprT2, std::multiplies<typename exprTraits<ExprT1>::ImagePixelT>, bitwise_or<typename exprTraits<ExprT1>::MaskPixelT>, variance_multiplies<typename exprTraits<ExprT1>::VariancePixelT> >(e1, e2); } For all (+,-,*,/) pixel operations, the mask operation is bitwise_or . Actually we do not have any operations with bitwise_and . Proposal: I'd propose a variant of the convolution operation that performs bitwise_and at the image and kernel pixel multiplication step. So that the kernel as a MaskedImage could carry a region "AND-mask" which would be bitwise_or-ed around a result pixel by the addition step. In this way, regions depending on the kernel value could be defined that determine the mask value of the output pixel.
            Hide
            swinbank John Swinbank added a comment -

            Great! Per discussion of 2019-10-15, we should:

            • Write another ticket to capture the work of RFCing the proposal above;
            • Claim this ticket as “done”.
            Show
            swinbank John Swinbank added a comment - Great! Per discussion of 2019-10-15, we should: Write another ticket to capture the work of RFCing the proposal above; Claim this ticket as “done”.
            Hide
            gkovacs Gabor Kovacs added a comment - - edited

            There is a  if (kval != 0) condition around the pixel multiplications that I've overlooked earlier. Nevertheless, for AL kernel solutions the kernel pixels are never exactly zero. (They may for other kernel classes, where an analytic function was not evaluated, but I don't fully see the implementation details. )

            Gabor Kovacs reports that this behavior is due to the mask planes being OR'd during kernel convolutions, even for pixels where the kernel weights are 0.

            Show
            gkovacs Gabor Kovacs added a comment - - edited There is a  if (kval != 0) condition around the pixel multiplications that I've overlooked earlier. Nevertheless, for AL kernel solutions the kernel pixels are never exactly zero. (They may for other kernel classes, where an analytic function was not evaluated, but I don't fully see the implementation details. ) Gabor Kovacs reports that this behavior is due to the mask planes being OR'd during kernel convolutions, even for pixels where the kernel weights are 0.

              People

              • Assignee:
                gkovacs Gabor Kovacs
                Reporter:
                ebellm Eric Bellm
                Watchers:
                Eric Bellm, Gabor Kovacs, John Swinbank
              • Votes:
                0 Vote for this issue
                Watchers:
                3 Start watching this issue

                Dates

                • Created:
                  Updated:
                  Resolved:

                  Summary Panel