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

Upgrades to overscan correction in ip_isr

    XMLWordPrintable

    Details

    • Type: Improvement
    • Status: Won't Fix
    • Resolution: Done
    • Fix Version/s: None
    • Component/s: ip_isr
    • Labels:
      None
    • Team:
      Data Release Production

      Description

      The issues in this ticket pertain to the function overscanCorrection in isr.py in repository ip_isr.

      When testing code from and HSC port, I was able to break the code in some ways which may occur in real observations. In the course of investigating the breakages I noticed several places the code could be improved to not only fix the breakages, but make it more robust in general.

      • If fitType is set to mean or median, appropriate masks should be used for clipping/defect identification; currently only the image portion is passed to afwMath.makeStatistics.
      • Expand the suspect bit mask to be applied to rows with defects, even if the fitType is mean or median (This may not be necessary, but there is no way to know the true value of the bias int the row)
      • If there are too few overscan pixels per row, the percentiles used for clipping are skewed, and high values are not appropriately masked out, perhaps use some information from surrounding rows, and/or ampMaskImage mask information. If high pixels are left in the collapsed bias array, they will affect subsequent fitting. This may additionally occur if an entire row is affected, and thus the median value of the row is the median of bad data.

      If the fit to the background level is poor, or in some way skewed it may be possible to introduce high frequency variation in the bias level of the sky background on a row by row basis. I propose to change the fitting process such that instead of collapsing the array down to one dimension before fitting for the bias function, we fit using all of the pixels in the overscan, with the pixels in a row representing a sample along the column dimension. Each of the pixels can be weighted in the joint fit according to the distance away from the median (or mean) value of the row (or some other weighting function; it may be important to down-weight the highest quartile more than the lowest for example). In this way cosmic rays and other isolated pixels will be "clipped" by being down weighted in the fit, and the fit overall should be more accurate and robust.

      Alternatively, some clipping may be necessary in the collapsed array, to ensure rows which contain bleeding are not mistakenly identified as signal. This issue can be demonstrated by inserting a high value or row into the unit test. OverscanCorrection treats this as a valid value or row and does not mask the row. Subsequent fits for bias are then affected.

      These issues may not be as important with a large number of pixels per row, but the boundary between when it would matter or not matter has not been explored. If we proactively fix the issue with a different routine we can avoid weird behavior to be fixed in future data releases.

        Attachments

          Issue Links

            Activity

            Hide
            krughoff Simon Krughoff added a comment -

            I'm all for improving the robustness of the ISR routines, but in this case I'm not sure I understand the issue entirely. The kinds of effects you mention should not affect the overscan. Since the overscan is not made up of physical pixels they can't have bleed trails or CRs.

            If we are worried about ringing in the solution due to sharp features in the overscan in the parallel direction, it seems like a better thing to do is to just subtract off the row-wise median rather than fit at all since those sharp features are real features in the bias.

            I agree that a scheme like the one described here would probably work, but I'm not sure we need it and it adds quite a bit of complexity.

            Show
            krughoff Simon Krughoff added a comment - I'm all for improving the robustness of the ISR routines, but in this case I'm not sure I understand the issue entirely. The kinds of effects you mention should not affect the overscan. Since the overscan is not made up of physical pixels they can't have bleed trails or CRs. If we are worried about ringing in the solution due to sharp features in the overscan in the parallel direction, it seems like a better thing to do is to just subtract off the row-wise median rather than fit at all since those sharp features are real features in the bias. I agree that a scheme like the one described here would probably work, but I'm not sure we need it and it adds quite a bit of complexity.
            Hide
            price Paul Price added a comment -

            The overscan may not be made up of physical pixels, but effects from the physical pixels can leak into the overscan. We have seen several hundred complete rows of overscan destroyed by a very bright star: https://hsc-jira.astro.princeton.edu/jira/browse/HSC-1199 . Of course, this is exacerbated by the fact that HSC doesn't clock the serials during integration, but it is certainly a real effect in real CCDs.

            Show
            price Paul Price added a comment - The overscan may not be made up of physical pixels, but effects from the physical pixels can leak into the overscan. We have seen several hundred complete rows of overscan destroyed by a very bright star: https://hsc-jira.astro.princeton.edu/jira/browse/HSC-1199 . Of course, this is exacerbated by the fact that HSC doesn't clock the serials during integration, but it is certainly a real effect in real CCDs.
            Hide
            mfisherlevine Merlin Fisher-Levine added a comment -

            Not wanting to be pedantic here, and whilst it doesn't happen very often, you can get cosmics in the overscan. I'm not saying it's a big issue or that it even needs correcting, but it is possible (and I've seen it). Whilst the overscan doesn't exactly correspond to physical pixels, physical pixel contents are still being clocked through whilst taking the data, so if a cosmic hits the edge of the sensor after the main readout but whilst the overscan is being taken then this will appear as a cosmic track in the overscan region itself.

            Show
            mfisherlevine Merlin Fisher-Levine added a comment - Not wanting to be pedantic here, and whilst it doesn't happen very often, you can get cosmics in the overscan. I'm not saying it's a big issue or that it even needs correcting, but it is possible (and I've seen it). Whilst the overscan doesn't exactly correspond to physical pixels, physical pixel contents are still being clocked through whilst taking the data, so if a cosmic hits the edge of the sensor after the main readout but whilst the overscan is being taken then this will appear as a cosmic track in the overscan region itself.
            Hide
            krughoff Simon Krughoff added a comment -

            O.K. These are both good points.

            This suggests to me that we need to add some functionality to mask pixels in the overscan if needed. We then also need to make the fitting robust to masked pixels in the overscan. Is the first piece captured anywhere?

            Show
            krughoff Simon Krughoff added a comment - O.K. These are both good points. This suggests to me that we need to add some functionality to mask pixels in the overscan if needed. We then also need to make the fitting robust to masked pixels in the overscan. Is the first piece captured anywhere?
            Hide
            price Paul Price added a comment -

            The SubaruIsrTask includes:

                def saturationDetection(self, exposure, amp, maskName="SAT"):
                    """Detect saturated pixels and mask them using mask plane "SAT", in place
             
                    @param[in,out]  exposure    exposure to process; only the amp DataSec is processed
                    @param[in]      amp         amplifier device data
                    """
                    if not self.checkIsAmp(amp):
                        raise RuntimeError("This method must be executed on an amp.")
                    for box in (amp.getDiskDataSec(), amp.getDiskBiasSec()):
                        dataView = afwImage.MaskedImageF(exposure.getMaskedImage(), box, afwImage.PARENT)
                        bad = numpy.where(dataView.getImage().getArray() > amp.getElectronicParams().getSaturationLevel())
                        dataView.getMask().getArray()[bad] = dataView.getMask().getPlaneBitMask(maskName)
            

            We may want to push this up to the parent IsrTask (and port it to the modern cameraGeom).

            Show
            price Paul Price added a comment - The SubaruIsrTask includes: def saturationDetection(self, exposure, amp, maskName="SAT"): """Detect saturated pixels and mask them using mask plane "SAT", in place   @param[in,out] exposure exposure to process; only the amp DataSec is processed @param[in] amp amplifier device data """ if not self.checkIsAmp(amp): raise RuntimeError("This method must be executed on an amp.") for box in (amp.getDiskDataSec(), amp.getDiskBiasSec()): dataView = afwImage.MaskedImageF(exposure.getMaskedImage(), box, afwImage.PARENT) bad = numpy.where(dataView.getImage().getArray() > amp.getElectronicParams().getSaturationLevel()) dataView.getMask().getArray()[bad] = dataView.getMask().getPlaneBitMask(maskName) We may want to push this up to the parent IsrTask (and port it to the modern cameraGeom).
            Hide
            swinbank John Swinbank added a comment -

            Christopher Waters I think you have looked at this code most recently, albeit primarily from a documentation/cleanup perspective. Do you have a feeling for how many of the issues raised in this ticket are still valid and/or urgent? Thanks!

            Show
            swinbank John Swinbank added a comment - Christopher Waters I think you have looked at this code most recently, albeit primarily from a documentation/cleanup perspective. Do you have a feeling for how many of the issues raised in this ticket are still valid and/or urgent? Thanks!
            Hide
            czw Christopher Waters added a comment -

            I believe most of the issues in this ticket are already resolved.  Working through the points as I see them:

            1. IsrTask now includes the SubaruIsrTask saturation detection, and performs this (and suspect pixel detection) prior to overscan correction.
            2. The point about MEAN and MEDIAN fitTypes possibly using a clipping could be implemented, but it seems like this is logically what the MEANCLIP fitType does (and I would be surprised if there was enough contamination sufficient to bias the median value).  The higher-order polynomial and spline fits do do this kind of clipping using a robust sigma estimate.
            3. In addition, the full MaskedImage is passed to the statistics function, which I assume means the masks (including saturation and suspect planes at this point, regardless of if clipping was performed) are included.  If this is not the default case for passing a MI to afwMath.makeStatistics(), I would like to fix that.
            4. Entire rows are not masked as suspect if a single pixel is found to be a defect, but for the fitTypes that include clipping these pixels would still be less than 3-sigma discrepant from the median.
            5. We are dependent on each row of overscan having enough good pixels that clipping can occur.  However, the weight of an individual row is low (<1% for HSC) in the higher order fits, so I don't see how smoothing will help much more.
            6. Collapsing the data perpendicular to the length of the overscan, making each row a sample in the fit, is what is already done.

            The major point that this seems to get to is "what happens if the overscan data isn't cleanly symmetric about the median/best fit line?".  The current overscan code will deal with this only as well as the current median/robust sigma clipping can do.  If we do expect that future data will have highly biased overscans, we can add a new fitType, but I haven't seen any of that in the somewhat random sample of HSC visit/detector images I've run through ISR with the debug code enabled.

            Show
            czw Christopher Waters added a comment - I believe most of the issues in this ticket are already resolved.  Working through the points as I see them: IsrTask now includes the SubaruIsrTask saturation detection, and performs this (and suspect pixel detection) prior to overscan correction. The point about MEAN and MEDIAN fitTypes possibly using a clipping could be implemented, but it seems like this is logically what the MEANCLIP fitType does (and I would be surprised if there was enough contamination sufficient to bias the median value).  The higher-order polynomial and spline fits do do this kind of clipping using a robust sigma estimate. In addition, the full MaskedImage is passed to the statistics function, which I assume means the masks (including saturation and suspect planes at this point, regardless of if clipping was performed) are included.  If this is not the default case for passing a MI to afwMath.makeStatistics(), I would like to fix that. Entire rows are not masked as suspect if a single pixel is found to be a defect, but for the fitTypes that include clipping these pixels would still be less than 3-sigma discrepant from the median. We are dependent on each row of overscan having enough good pixels that clipping can occur.  However, the weight of an individual row is low (<1% for HSC) in the higher order fits, so I don't see how smoothing will help much more. Collapsing the data perpendicular to the length of the overscan, making each row a sample in the fit, is what is already done. The major point that this seems to get to is "what happens if the overscan data isn't cleanly symmetric about the median/best fit line?".  The current overscan code will deal with this only as well as the current median/robust sigma clipping can do.  If we do expect that future data will have highly biased overscans, we can add a new fitType, but I haven't seen any of that in the somewhat random sample of HSC visit/detector images I've run through ISR with the debug code enabled.
            Hide
            swinbank John Swinbank added a comment -

            Thank you for the comprehensive reply Christopher Waters!

            Given the above, I propose to close this ticket as “won't fix”. Please reopen if you disagree.

            Show
            swinbank John Swinbank added a comment - Thank you for the comprehensive reply Christopher Waters ! Given the above, I propose to close this ticket as “won't fix”. Please reopen if you disagree.

              People

              Assignee:
              czw Christopher Waters
              Reporter:
              nlust Nate Lust
              Watchers:
              Christopher Waters, John Swinbank, Merlin Fisher-Levine, Nate Lust, Paul Price, Simon Krughoff
              Votes:
              0 Vote for this issue
              Watchers:
              6 Start watching this issue

                Dates

                Created:
                Updated:
                Resolved:

                  Jenkins Builds

                  No builds found.