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

Possible masking issue in ip_isr

    XMLWordPrintable

Details

    • Bug
    • Status: Invalid
    • Resolution: Done
    • None
    • ip_isr
    • None
    • Data Release Production
    • No

    Description

      mfisherlevine reported today that the RubinTV processing was claiming N>0 pixels were marked as BAD, despite that processing not applying defects (or much other ISR steps beyond overscan).  When I ran the example attached on the RSP, I found a similar result, with lsst.isr INFO: Set 9598 BAD pixels to 245.000000. appearing in the log.

      Running the same code in the terminal with a pdb breakpoint resulted in no masked pixels, even when I switched versions from the w.2023.16 build to the older version running at the RSP (g8db5811388+2736d543e7 from mid-February it seems).  Without defects, I think the only code that sets BAD masks is in updateVariance, which sets that when the image is negative (which shouldn't be the case with this minimal ISR processing).

      Attachments

        Activity

          erykoff Eli Rykoff added a comment -

          I cannot replicate on the RSP with w_2023_16 or on sdfrome001. Or rather, I get the following which I think is fine?

          In [6]: import numpy as np
          In [7]: np.where(exp.mask.array > 0)
          Out[7]: (array([], dtype=int64), array([], dtype=int64))
          

          erykoff Eli Rykoff added a comment - I cannot replicate on the RSP with w_2023_16 or on sdfrome001. Or rather, I get the following which I think is fine? In [6]: import numpy as np In [7]: np.where(exp.mask.array > 0) Out[7]: (array([], dtype=int64), array([], dtype=int64))

          You have to turn off MEDIAN_PER_ROW to trigger the bug, for some reason that stops it setting the BAD pixels.

          mfisherlevine Merlin Fisher-Levine added a comment - You have to turn off MEDIAN_PER_ROW to trigger the bug, for some reason that stops it setting the BAD pixels.
          erykoff Eli Rykoff added a comment -

          As it is, it's subtracting off a bad overscan model, and at the edge of the chip the signal drops down, so subtracting the wrong overscan leads to negative signal, and it can't compute the variance so it marks them bad. (If you let negative variance through our pipelines BAD THINGS happen, and also, what does negative signal mean?)

          If you don't care about this, set isrConfig.doVariance = False and it won't compute the variance plane and won't complain about negative pixels.

          erykoff Eli Rykoff added a comment - As it is, it's subtracting off a bad overscan model, and at the edge of the chip the signal drops down, so subtracting the wrong overscan leads to negative signal, and it can't compute the variance so it marks them bad. (If you let negative variance through our pipelines BAD THINGS happen, and also, what does negative signal mean?) If you don't care about this, set isrConfig.doVariance = False and it won't compute the variance plane and won't complain about negative pixels.

          This is certainly the explanation, and I think it's the right answer, it just gives me a little pause. But I'll take Eli's word that this is the right thing to do. I guess we mark as Invalid/Won't Fix then?

          mfisherlevine Merlin Fisher-Levine added a comment - This is certainly the explanation, and I think it's the right answer, it just gives me a little pause. But I'll take Eli's word that this is the right thing to do. I guess we mark as Invalid/Won't Fix then?
          erykoff Eli Rykoff added a comment -

          I really think that this is the intended behavior, and if you don't want negative-signal pixels to be flagged you can either (a) turn off the variance computation, or (b) use a better overscan model that doesn't create negative pixels.

          An argument could be made to have yet another config doMaskNegativePixels that is separate from the variance plane computation, which must be true if doVariance is true to make it even more explicit, but I also don't think we necessarily need more do flags.

          erykoff Eli Rykoff added a comment - I really think that this is the intended behavior, and if you don't want negative-signal pixels to be flagged you can either (a) turn off the variance computation, or (b) use a better overscan model that doesn't create negative pixels. An argument could be made to have yet another config doMaskNegativePixels that is separate from the variance plane computation, which must be true if doVariance is true to make it even more explicit, but I also don't think we necessarily need more do flags.
          erykoff Eli Rykoff added a comment -

          Yikes, it's already there, but without the do name... https://github.com/lsst/ip_isr/blob/main/python/lsst/ip/isr/isrTask.py#L559-L565

              maskNegativeVariance = pexConfig.Field(
                  dtype=bool,
                  default=True,
                  doc="Mask pixels that claim a negative variance?  This likely indicates a failure "
                  "in the measurement of the overscan at an edge due to the data falling off faster "
                  "than the overscan model can account for it."
              )
          

          erykoff Eli Rykoff added a comment - Yikes, it's already there, but without the do name... https://github.com/lsst/ip_isr/blob/main/python/lsst/ip/isr/isrTask.py#L559-L565 maskNegativeVariance = pexConfig.Field( dtype = bool , default = True , doc = "Mask pixels that claim a negative variance? This likely indicates a failure " "in the measurement of the overscan at an edge due to the data falling off faster " "than the overscan model can account for it." )

          People

            Unassigned Unassigned
            czw Christopher Waters
            Christopher Waters, Eli Rykoff, Merlin Fisher-Levine
            Votes:
            0 Vote for this issue
            Watchers:
            3 Start watching this issue

            Dates

              Created:
              Updated:
              Resolved:

              Jenkins

                No builds found.