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
| Field | Original Value | New Value |
|---|---|---|
| Attachment | dm37022-czw.png [ 64709 ] |
| Attachment | dm37022-czw-1.png [ 64710 ] |
| Attachment | dm37022-czw.png [ 64709 ] |
| Status | To Do [ 10001 ] | In Progress [ 3 ] |
| Epic Link |
|
|
| Team | Data Release Production [ 10301 ] |
| Reviewers | Craig Lage [ cslage ] | Eli Rykoff [ erykoff ] |
| Status | In Progress [ 3 ] | In Review [ 10004 ] |
I worry a lot about the lack of tests ...
| Status | In Review [ 10004 ] | Reviewed [ 10101 ] |
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.
| Story Points | 1 |
| Resolution | Done [ 10000 ] | |
| Status | Reviewed [ 10101 ] | Done [ 10002 ] |
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-34989hasn'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".