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

Switch DECam overscan correction to median per row

    XMLWordPrintable

    Details

      Description

      update Jan 2022

      Original ticket title: Investigate amp background level offsets in DECam postISRCCDs

      This ticket updates the DECam ISR overscan correction config parameter fitType='MEDIAN_PER_ROW'. A large body of evidence (see comments below) point towards this being a superior approach at handling overscan corrections with DECam. This also brings Science Pipelines data processing in-line with the DECam Community Pipeline, which also fits per row.

      Original ticket efforts by Kenneth Herner, original description below:

      If you scroll through here, the problem is immediately apparent

      https://lsst.ncsa.illinois.edu/~lskelvin/hits2014/

      Hat-tip to Lee Kelvin for noticing this problem and plotting the images. This feels very familiar to when ISR order of operations was inconsistent regarding when assembleCcd happens, but I don't remember the resolution.

      Possibly relevant and/or wrong: https://community.lsst.org/t/a-change-to-the-order-of-operations-for-isr/3060 

        Attachments

        1. merian_bias_01.png
          merian_bias_01.png
          821 kB
        2. merian_bias_35.png
          merian_bias_35.png
          909 kB
        3. merian_bias_45.png
          merian_bias_45.png
          800 kB
        4. merian_flat_01.png
          merian_flat_01.png
          2.97 MB
        5. merian_flat_05.png
          merian_flat_05.png
          2.60 MB
        6. merian_flat_25.png
          merian_flat_25.png
          3.07 MB
        7. merian_flat_35.png
          merian_flat_35.png
          4.36 MB
        8. merian_flat_45.png
          merian_flat_45.png
          2.66 MB
        9. merian_flat_55.png
          merian_flat_55.png
          3.99 MB

          Issue Links

            Activity

            Hide
            mrawls Meredith Rawls added a comment -

            Meredith, Chris, and Lee plan to work together on this during the 11am PDT June 9 pair coding.

            Show
            mrawls Meredith Rawls added a comment - Meredith, Chris, and Lee plan to work together on this during the 11am PDT June 9 pair coding.
            Hide
            mrawls Meredith Rawls added a comment -

            We investigated today and were able to characterize the problem as well as come up with a potential solution.

            We see discontinuities in postISRCCDs both left-right (amps) and top-bottom.

            We think that if we configure isrTask to use overscan.fitType='MEDIAN_PER_ROW' (instead of the default fittype='MEDIAN') that will fix the top-bottom level mismatch. This needs to be done on ALL raws, calib and science frames, when re-running ISR, and probably in the crosstalk-prep step as well. https://github.com/lsst/ip_isr/blob/master/python/lsst/ip/isr/overscan.py#L48

            We think that if we configure cpFlatNormalizationTask to use level='AMP' (instead of the default level='DETECTOR') this will fix the left-right amp-level mismatch. https://github.com/lsst/cp_pipe/blob/master/python/lsst/cp/pipe/cpFlatNormTask.py#L165

            The next step is to try these configs in new DECam reruns entirely from scratch and see how things look. 

            Show
            mrawls Meredith Rawls added a comment - We investigated today and were able to characterize the problem as well as come up with a potential solution. We see discontinuities in postISRCCDs both left-right (amps) and top-bottom. We think that if we configure isrTask to use overscan.fitType='MEDIAN_PER_ROW' (instead of the default fittype='MEDIAN') that will fix the top-bottom level mismatch. This needs to be done on ALL raws, calib and science frames, when re-running ISR, and probably in the crosstalk-prep step as well.  https://github.com/lsst/ip_isr/blob/master/python/lsst/ip/isr/overscan.py#L48 We think that if we configure cpFlatNormalizationTask to use level='AMP' (instead of the default level='DETECTOR') this will fix the left-right amp-level mismatch.  https://github.com/lsst/cp_pipe/blob/master/python/lsst/cp/pipe/cpFlatNormTask.py#L165 The next step is to try these configs in new DECam reruns entirely from scratch and see how things look. 
            Hide
            czw Christopher Waters added a comment -

            To add an extra note: updating the overscan fitType will make all of the `crosstalkSources`/pre-generated overscan subtracted images used for inter-chip crosstalk slightly wrong.  Given the scale of the crosstalk (~1e5-1e-4), and the scale of the differences (~1 ADU), this is probably not worth rerunning, but I wanted to make sure it was mentioned at least. 

            This may show up as slight roll downs at the top of crosstalk corrected images (run with median-per-row that eliminates the bright roll up at large y) as the old crosstalkSources (which were run with median, and so still have that bright roll up) are scaled and subtracted off.  Again, my quick calculation suggests this should still be smaller than an ADU (1e-4 * 2500 = 0.25), but I wanted to bring up the point.

            Show
            czw Christopher Waters added a comment - To add an extra note: updating the overscan fitType will make all of the `crosstalkSources`/pre-generated overscan subtracted images used for inter-chip crosstalk slightly wrong.  Given the scale of the crosstalk (~1e5-1e-4), and the scale of the differences (~1 ADU), this is probably not worth rerunning, but I wanted to make sure it was mentioned at least.  This may show up as slight roll downs at the top of crosstalk corrected images (run with median-per-row that eliminates the bright roll up at large y) as the old crosstalkSources (which were run with median, and so still have that bright roll up) are scaled and subtracted off.  Again, my quick calculation suggests this should still be smaller than an ADU (1e-4 * 2500 = 0.25), but I wanted to bring up the point.
            Hide
            lskelvin Lee Kelvin added a comment - - edited

            I've been able to re-run the $CP_PIPE_DIR/pipelines/cpBias.yaml pipeline to regenerate master biases for my Merian DECam data using the config option isr:overscan.fitType='MEDIAN_PER_ROW'. The plots below show a few examples of the before (left) and after (right):

            The biases with the new config option in place appear much healthier, resolving the upper/lower discontinuity, and also the left/right amp-to-amp discontinuity.

            Plots for all 62 detectors can be found linked:

            I'll continue rerunning my test data to see what else changes - next is generating crosstalk sources and building flat frames.

            Show
            lskelvin Lee Kelvin added a comment - - edited I've been able to re-run the $CP_PIPE_DIR/pipelines/cpBias.yaml pipeline to regenerate master biases for my Merian DECam data using the config option isr:overscan.fitType='MEDIAN_PER_ROW' . The plots below show a few examples of the before (left) and after (right): The biases with the new config option in place appear much healthier, resolving the upper/lower discontinuity, and also the left/right amp-to-amp discontinuity. Plots for all 62 detectors can be found linked: current defaults: https://tigress-web.princeton.edu/~lkelvin/merian/bias-det/ new configuration: https://tigress-web.princeton.edu/~lkelvin/merian/bias-det-mpr/ I'll continue rerunning my test data to see what else changes - next is generating crosstalk sources and building flat frames.
            Hide
            lskelvin Lee Kelvin added a comment - - edited

            Following on from above, I've re-run the $AP_PIPE_DIR/pipelines/DarkEnergyCamera/RunIsrForCrosstalkSources.yaml pipeline to regenerate crosstalk sources using the MEDIAN_PER_ROW fitType, and then regenerated flats using the $CP_PIPE_DIR/pipelines/DarkEnergyCamera/cpFlat.yaml pipeline.

            Including the original (default) flats data I have available, I now have three flats outputs:

            1. isr:overscan.fitType='MEDIAN' AND cpFlatNorm:level='DETECTOR' (current default)
            2. isr:overscan.fitType='MEDIAN_PER_ROW' AND cpFlatNorm:level='DETECTOR'
            3. isr:overscan.fitType='MEDIAN_PER_ROW' AND cpFlatNorm:level='AMP'

            The plots attached here show comparisons for a few selected DECam detectors:


            I'm unsure which of these flats configurations look the healthiest, and would be very keen to hear from others who are more experienced here. From the bias testing above, it seems that MEDIAN_PER_ROW is optimal for DECam, but the impact of switching cpFlatNormalizationTask from per-detector to per-amp varying results, with a large variation as a function of detector.

            Show
            lskelvin Lee Kelvin added a comment - - edited Following on from above, I've re-run the $AP_PIPE_DIR/pipelines/DarkEnergyCamera/RunIsrForCrosstalkSources.yaml pipeline to regenerate crosstalk sources using the MEDIAN_PER_ROW fitType, and then regenerated flats using the $CP_PIPE_DIR/pipelines/DarkEnergyCamera/cpFlat.yaml pipeline. Including the original (default) flats data I have available, I now have three flats outputs: isr:overscan.fitType='MEDIAN' AND cpFlatNorm:level='DETECTOR' (current default) isr:overscan.fitType='MEDIAN_PER_ROW' AND cpFlatNorm:level='DETECTOR' isr:overscan.fitType='MEDIAN_PER_ROW' AND cpFlatNorm:level='AMP' The plots attached here show comparisons for a few selected DECam detectors: I'm unsure which of these flats configurations look the healthiest, and would be very keen to hear from others who are more experienced here. From the bias testing above, it seems that MEDIAN_PER_ROW is optimal for DECam, but the impact of switching cpFlatNormalizationTask from per-detector to per-amp varying results, with a large variation as a function of detector.
            Hide
            mrawls Meredith Rawls added a comment -

            Before closing this ticket, please remove references to it in the obs_decam docs, specifically the using-obs-decam.rst file written for DM-24913.

            Show
            mrawls Meredith Rawls added a comment - Before closing this ticket, please remove references to it in the obs_decam docs, specifically the using-obs-decam.rst file written for DM-24913 .
            Hide
            lskelvin Lee Kelvin added a comment - - edited

            Update in Jan 2022: This ticket has been re-focused to only look at updating the overscan fitType to median per row. The tests above seemed to support this change, however, I couldn't see any consistent evidence for a strong preference between the flat level being at the detector or amp setting. To that end, I propose that this ticket focuses on the fitType setting alone, and a future ticket can look again at level if relevant.

            Meredith Rawls, Kenneth Herner, I'm not sure what impact this config change will have on any existing DECam data sets that you regularly process? From Christopher Waters's comment above, it's clear that this update will require calibs to be re-generated (including the crosstalk sources). This is something we plan on doing with the Merian data reductions on-going here at Princeton, but I wanted to ping you both to ask if there are any issues from your side in making this config change in the short term?

            Show
            lskelvin Lee Kelvin added a comment - - edited Update in Jan 2022: This ticket has been re-focused to only look at updating the overscan fitType to median per row. The tests above seemed to support this change, however, I couldn't see any consistent evidence for a strong preference between the flat level being at the detector or amp setting. To that end, I propose that this ticket focuses on the fitType setting alone, and a future ticket can look again at level if relevant. Meredith Rawls , Kenneth Herner , I'm not sure what impact this config change will have on any existing DECam data sets that you regularly process? From Christopher Waters 's comment above, it's clear that this update will require calibs to be re-generated (including the crosstalk sources). This is something we plan on doing with the Merian data reductions on-going here at Princeton, but I wanted to ping you both to ask if there are any issues from your side in making this config change in the short term?
            Hide
            mrawls Meredith Rawls added a comment -

            My most recent DECam processing for AP used manually-built calibs that had both the config overrides mentioned on this ticket (overscan set to median-per-row and flat normalization set to the amp level). The overscan median-per-row is almost certainly what we want, and I have yet to encounter any glaring issues traceable to flat normalization. Please proceed with updating defaults!

            Show
            mrawls Meredith Rawls added a comment - My most recent DECam processing for AP used manually-built calibs that had both the config overrides mentioned on this ticket (overscan set to median-per-row and flat normalization set to the amp level). The overscan median-per-row is almost certainly what we want, and I have yet to encounter any glaring issues traceable to flat normalization. Please proceed with updating defaults!
            Hide
            kherner Kenneth Herner added a comment -

            The Saha Bulge calibs I created for PREOPS-597 also had overscan set to median-per-row, so they won't be affected. Otherwise the datasets I'm using for the diffim settings comparisons are actually HSC images. From my perspective, fire away.

             

            General comment: After discussing with Meredith Rawls, Lee Kelvin, and Ian Sullivan, the plan is to split this ticket up. The existing ticket will cover only this change to the overscan setting, and I will open a new ticket for doing the amp vs. detector investigation and assign that to myself.

            Show
            kherner Kenneth Herner added a comment - The Saha Bulge calibs I created for PREOPS-597 also had overscan set to median-per-row, so they won't be affected. Otherwise the datasets I'm using for the diffim settings comparisons are actually HSC images. From my perspective, fire away.   General comment: After discussing with Meredith Rawls , Lee Kelvin , and Ian Sullivan , the plan is to split this ticket up. The existing ticket will cover only this change to the overscan setting, and I will open a new ticket for doing the amp vs. detector investigation and assign that to myself.
            Hide
            lskelvin Lee Kelvin added a comment - - edited

            Thanks Meredith Rawls and Kenneth Herner, that's good to know!

            Link to Jenkins: https://ci.lsst.codes/blue/organizations/jenkins/stack-os-matrix/detail/stack-os-matrix/35639/pipeline

            Build products used for Jenkins are: lsst_distrib lsst_ci ap_verify_ci_hits2015. If there's another package that should be added to the build products to test this DECam config change, please do let me know!

            Update: I have subsequently learned that adding ap_verify_ci_hits2015 to the list of build products should not make a difference - the ap_verify datasets are not scons-buildable. With that said, it shouldn't have any negative impact either, so the above Jenkins is still valid.

            Show
            lskelvin Lee Kelvin added a comment - - edited Thanks Meredith Rawls and Kenneth Herner , that's good to know! Link to Jenkins: https://ci.lsst.codes/blue/organizations/jenkins/stack-os-matrix/detail/stack-os-matrix/35639/pipeline Build products used for Jenkins are: lsst_distrib lsst_ci ap_verify_ci_hits2015 . If there's another package that should be added to the build products to test this DECam config change, please do let me know! Update: I have subsequently learned that adding ap_verify_ci_hits2015 to the list of build products should not make a difference - the ap_verify datasets are not scons-buildable. With that said, it shouldn't have any negative impact either, so the above Jenkins is still valid.
            Hide
            kherner Kenneth Herner added a comment -

            Changes look fine; Jenkins running.

            Show
            kherner Kenneth Herner added a comment - Changes look fine; Jenkins running.
            Hide
            kherner Kenneth Herner added a comment -

            I created DM-33126 to continue work on the still-open question of level settings in cpFlatNormalization.

            Show
            kherner Kenneth Herner added a comment - I created  DM-33126 to continue work on the still-open question of level settings in cpFlatNormalization.
            Hide
            lskelvin Lee Kelvin added a comment -

            Jenkins picked up a couple of hard coded unit test values which also needed to be updated. I've updated these, pushed to the PR, and re-ran Jenkins here: https://ci.lsst.codes/blue/organizations/jenkins/stack-os-matrix/detail/stack-os-matrix/35645/pipeline/47

            Show
            lskelvin Lee Kelvin added a comment - Jenkins picked up a couple of hard coded unit test values which also needed to be updated. I've updated these, pushed to the PR, and re-ran Jenkins here: https://ci.lsst.codes/blue/organizations/jenkins/stack-os-matrix/detail/stack-os-matrix/35645/pipeline/47

              People

              Assignee:
              lskelvin Lee Kelvin
              Reporter:
              mrawls Meredith Rawls
              Reviewers:
              Kenneth Herner
              Watchers:
              Christopher Waters, Dino Bektesevic, Eric Bellm, Ian Sullivan, Kenneth Herner, Lee Kelvin, Meredith Rawls
              Votes:
              0 Vote for this issue
              Watchers:
              7 Start watching this issue

                Dates

                Created:
                Updated:

                  CI Builds

                  No builds found.