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

overscan.py parallel overscan sigma clip is really a threshold clip

    XMLWordPrintable

    Details

    • Type: Bug
    • Status: Done
    • Resolution: Done
    • Fix Version/s: None
    • Component/s: ip_isr
    • Labels:
      None
    • Story Points:
      1
    • Epic Link:
    • Team:
      Data Release Production
    • Urgent?:
      No

      Description

      I found when trying to run the parallel overscan in overscan.py, that most parallel overscan values were being masked out. I plotted a histogram of the values, and they look reasonable, and only a few should be masked out. See : Parallel_Overscan_Values_Amp_C10_16Nov22.pdf
      The problem is that at this line:
      https://github.com/lsst/ip_isr/blob/e3fba828a5a18f981ebfd0a6426b864e7888fb3f/python/lsst/ip/isr/overscan.py#L221
      The threshold is treated as a number of sigma, but at this line:
      https://github.com/lsst/ip_isr/blob/e3fba828a5a18f981ebfd0a6426b864e7888fb3f/python/lsst/ip/isr/isrFunctions.py#L126
      the threshold is a scalar value. When I set isrConfig.overscan.numSigmaClip=30.0, then it behaved as it should. I don't know whether the right fix is to change the name of numSigmaClip or to re-write makeThresholdMask.

        Attachments

          Activity

          Hide
          czw Christopher Waters added a comment - - edited

          I'm changing this to continue to use numSigmaClip and makeThresholdMask as they are, but changing the threshold to use {{numSigmaClip * serialOverscanSigmaResidual.  }}This is slightly more conservative than the value of 30 (~21-24 ADU at 3-sigma), but fixes the code to be consistent with what the config options say.

          My residual images with this, setting doParallelOverscan=True and confirming that it's running overscan.fitType=MEDIAN_PER_ROW (as DM-34989 hasn't hit the weekly yet) largely fixes the bar in C10.  For my comparison to the plot on slack, I did need to disable doBias.  Bias subtraction runs after the overscan subtraction, so applying the existing bias for this data imprints a negative bias "wave".

          Show
          czw Christopher Waters added a comment - - edited I'm changing this to continue to use numSigmaClip  and makeThresholdMask as they are, but changing the threshold to use {{numSigmaClip * serialOverscanSigmaResidual.  }}This is slightly more conservative than the value of 30 (~21-24 ADU at 3-sigma), but fixes the code to be consistent with what the config options say. My residual images with this, setting doParallelOverscan=True  and confirming that it's running overscan.fitType=MEDIAN_PER_ROW  (as DM-34989 hasn't hit the weekly yet) largely fixes the bar in C10.  For my comparison to the plot on slack, I did need to disable doBias .  Bias subtraction runs after the overscan subtraction, so applying the existing bias for this data imprints a negative bias "wave".
          Hide
          erykoff Eli Rykoff added a comment -

          I worry a lot about the lack of tests ...

          Show
          erykoff Eli Rykoff added a comment - I worry a lot about the lack of tests ...
          Hide
          czw Christopher Waters added a comment -

          Agree on the tests, but this is a general issue, so I've made DM-37039 to do another pass through ip_isr's tests to improve coverage and results.

          Show
          czw Christopher Waters added a comment - Agree on the tests, but this is a general issue, so I've made  DM-37039 to do another pass through ip_isr 's tests to improve coverage and results.

            People

            Assignee:
            czw Christopher Waters
            Reporter:
            cslage Craig Lage
            Reviewers:
            Eli Rykoff
            Watchers:
            Christopher Waters, Craig Lage, Eli Rykoff
            Votes:
            0 Vote for this issue
            Watchers:
            3 Start watching this issue

              Dates

              Created:
              Updated:
              Resolved:

                Jenkins

                No builds found.