# Switch DECam overscan correction to median per row

XMLWordPrintable

#### Details

• Type: Story
• Status: Done
• Resolution: Done
• Fix Version/s: None
• Component/s: None
• Labels:
• Story Points:
4
• Team:
Data Release Production
• Urgent?:
No

#### 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
821 kB
2. merian_bias_35.png
909 kB
3. merian_bias_45.png
800 kB
4. merian_flat_01.png
2.97 MB
5. merian_flat_05.png
2.60 MB
6. merian_flat_25.png
3.07 MB
7. merian_flat_35.png
4.36 MB
8. merian_flat_45.png
2.66 MB
9. merian_flat_55.png
3.99 MB

#### Activity

No builds found.
Meredith Rawls created issue -
Field Original Value New Value
Link This issue relates to DM-30495 [ DM-30495 ]
 Link This issue relates to DM-15152 [ DM-15152 ]
 Labels SciencePipelines SciencePipelines ap-analysis
 Assignee Meredith Rawls [ mrawls ]
 Assignee Meredith Rawls [ mrawls ]
Hide
Meredith Rawls added a comment -

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

Show
Meredith Rawls added a comment - Meredith, Chris, and Lee plan to work together on this during the 11am PDT June 9 pair coding.
Hide
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
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
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
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.
 Attachment merian_bias_01.png [ 50087 ] Attachment merian_bias_35.png [ 50088 ] Attachment merian_bias_45.png [ 50089 ]
Hide
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 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.
 Link This issue relates to DM-30703 [ DM-30703 ]
 Attachment merian_flat_01.png [ 50158 ]
 Attachment merian_flat_05.png [ 50159 ]
 Attachment merian_flat_25.png [ 50160 ]
 Attachment merian_flat_35.png [ 50161 ]
 Attachment merian_flat_45.png [ 50162 ]
 Attachment merian_flat_55.png [ 50163 ]
Hide
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
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.
 Attachment merian_flat_01.png [ 50158 ]
 Attachment merian_flat_05.png [ 50159 ]
 Attachment merian_flat_25.png [ 50160 ]
 Attachment merian_flat_35.png [ 50161 ]
 Attachment merian_flat_45.png [ 50162 ]
 Attachment merian_flat_55.png [ 50163 ]
 Attachment merian_flat_01.png [ 50164 ] Attachment merian_flat_05.png [ 50165 ] Attachment merian_flat_25.png [ 50166 ] Attachment merian_flat_35.png [ 50167 ] Attachment merian_flat_45.png [ 50168 ] Attachment merian_flat_55.png [ 50169 ]
 Assignee Meredith Rawls [ mrawls ]
 Assignee Meredith Rawls [ mrawls ] Kenneth Herner [ kherner ]
 Epic Link DM-30436 [ 504824 ] Sprint AP F21-2 (July) [ 1102 ] Team Alert Production [ 10300 ]
 Sprint AP F21-2 (July) [ 1102 ] AP F21-3 (August) [ 1109 ]
 Rank Ranked higher
Hide
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
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 .
 Sprint AP F21-3 (August) [ 1109 ] AP F21-4 (September) [ 1113 ]
 Rank Ranked higher
 Sprint AP F21-4 (September) [ 1113 ] AP F21-5 (October) [ 1119 ]
 Rank Ranked higher
 Epic Link DM-30436 [ 504824 ] DM-30501 [ 510159 ]
 Sprint AP F21-5 (October) [ 1119 ] AP F21-6 (November) [ 1124 ]
 Rank Ranked lower
 Sprint AP F21-6 (November) [ 1124 ] AP S22-1 (December) [ 1126 ]
 Rank Ranked lower
 Epic Link DM-30501 [ 510159 ] DM-30502 [ 510160 ]
 Sprint AP S22-1 (December) [ 1126 ] AP S22-2 (January) [ 1130 ]
 Rank Ranked lower
 Link This issue relates to DM-33096 [ DM-33096 ]
 Link This issue relates to DM-33096 [ DM-33096 ]
 Epic Link DM-30502 [ 510160 ] DM-30543 [ 511201 ] Sprint AP S22-2 (January) [ 1130 ] Story Points 2 4 Team Alert Production [ 10300 ] Data Release Production [ 10301 ] Assignee Kenneth Herner [ kherner ] Lee Kelvin [ lskelvin ] Description If you scroll through here, the problem is immediately apparent [https://lsst.ncsa.illinois.edu/~lskelvin/hits2014/] Hat-tip to [~lskelvin] 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] {panel:title=update} 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 [~kherner], original description below: {panel} If you scroll through here, the problem is immediately apparent [https://lsst.ncsa.illinois.edu/~lskelvin/hits2014/] Hat-tip to [~lskelvin] 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] Issue Type Bug [ 1 ] Story [ 10001 ] Summary Investigate amp background level offsets in DECam postISRCCDs Switch DECam overscan correction to median per row
 Reviewers Kenneth Herner [ kherner ]
 Status To Do [ 10001 ] In Progress [ 3 ]
 Description {panel:title=update} 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 [~kherner], original description below: {panel} If you scroll through here, the problem is immediately apparent [https://lsst.ncsa.illinois.edu/~lskelvin/hits2014/] Hat-tip to [~lskelvin] 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] {panel:title=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 [~kherner], original description below: {panel} If you scroll through here, the problem is immediately apparent [https://lsst.ncsa.illinois.edu/~lskelvin/hits2014/] Hat-tip to [~lskelvin] 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]
Hide
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
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
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
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
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
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
Lee Kelvin added a comment - - edited

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

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
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.
 Status In Progress [ 3 ] In Review [ 10004 ]
 Link This issue relates to DM-33096 [ DM-33096 ]
Hide
Kenneth Herner added a comment -

Changes look fine; Jenkins running.

Show
Kenneth Herner added a comment - Changes look fine; Jenkins running.
 Status In Review [ 10004 ] Reviewed [ 10101 ]
 Link This issue split to DM-33126 [ DM-33126 ]
Hide
Kenneth Herner added a comment -

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

Show
Kenneth Herner added a comment - I created  DM-33126 to continue work on the still-open question of level settings in cpFlatNormalization.
Hide
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
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
Hide
Lee Kelvin added a comment -

Thank you again to Kenneth Herner for your review, and Robert Lupton for taking a closer look under-the-hood. I rebased this ticket onto main, and re-ran Jenkins to make sure nothing fails. Branch merged and deleted, thanks again all.

Show
Lee Kelvin added a comment - Thank you again to Kenneth Herner for your review, and Robert Lupton for taking a closer look under-the-hood. I rebased this ticket onto main, and re-ran Jenkins to make sure nothing fails . Branch merged and deleted, thanks again all.
 Resolution Done [ 10000 ] Status Reviewed [ 10101 ] Done [ 10002 ]
 Link This issue relates to DM-35252 [ DM-35252 ]

#### People

Assignee:
Lee Kelvin
Reporter:
Meredith Rawls
Reviewers:
Kenneth Herner
Watchers:
Christopher Waters, Dino Bektesevic, Eric Bellm, Ian Sullivan, Kenneth Herner, Lee Kelvin, Meredith Rawls