Details
-
Type:
Improvement
-
Status: Done
-
Resolution: Done
-
Fix Version/s: None
-
Labels:None
-
Story Points:1
-
Epic Link:
-
Team:Data Release Production
Description
While debugging DM-27458, I noticed that the reason for many of the failures was that some images had few or no good pixels, and this caused the FFT algorithm that calculates the covariances to fail. While some amps have some defects, most pixels should be good. I tracked down one failure to raft R11, sensor S12, detector=41, and expId=3020110200190. The calibration data and postISRCCD images are in
/project/shared/BOT/rerun/cslage/PTC_LSSTCAM_12673
|
I queried the number of good pixels (good meaning the mask plane is 0) by amp, and obtained the following:
C10 0 |
C11 0 |
C12 0 |
C13 0 |
C14 0 |
C15 0 |
C16 0 |
C17 0 |
C07 3539 |
C06 614 |
C05 11 |
C04 78 |
C03 11 |
C02 20657 |
C01 974784 |
C00 409729 |
There should be about 1 million good pixels/amp, so only C01 looks normal. I then compared mask plane and image plane in a good region between C01(good) and C06(bad):
C06 Mask Good Region [6 6 6 6 6] |
C06 Image Good Region [99972.336 99972.336 99972.336 99972.336 99972.336] |
C01 Mask Good Region [0 0 0 0 0] |
C01 Image Good Region [104243.4 106106.586 105608.06 105036.266 104154.09 ] |
|
|
dict_items([('BAD', 0), ('CR', 3), ('DETECTED', 5), ('DETECTED_NEGATIVE', 6), ('EDGE', 4), ('INTRP', 2), ('NO_DATA', 8), ('SAT', 1), ('SUSPECT', 7), ('UNMASKEDNAN', 9)]) |
So the bad pixels have mask plane=6, which is 'SAT', and 'INTRP', so they have been interpreted as saturated. This isn't right, beacuse at ~100K ADU, they shouldn't be saturated. So I looked at obs_lsst/polict/lsstCam/R11.yaml for S12, and it shows:
S12 :
|
C10 : { gain : 0.699516, readNoise : 4.745807, saturation : 135968.062500 } |
C11 : { gain : 0.699235, readNoise : 4.708802, saturation : 136083.000000 } |
C12 : { gain : 0.695590, readNoise : 4.748249, saturation : 135391.828125 } |
C13 : { gain : 0.690748, readNoise : 4.987112, saturation : 134143.984375 } |
C14 : { gain : 0.696782, readNoise : 5.019731, saturation : 135183.234375 } |
C15 : { gain : 0.689901, readNoise : 5.063357, saturation : 134983.390625 } |
C16 : { gain : 0.693390, readNoise : 4.898793, saturation : 135653.421875 } |
C17 : { gain : 0.698506, readNoise : 4.976052, saturation : 135490.921875 } |
C07 : { gain : 0.721271, readNoise : 4.937364, saturation : 135981.453125 } |
C06 : { gain : 0.715233, readNoise : 4.710922, saturation : 134819.281250 } |
C05 : { gain : 0.713144, readNoise : 4.725629, saturation : 134069.796875 } |
C04 : { gain : 0.713045, readNoise : 4.748403, saturation : 134037.093750 } |
C03 : { gain : 0.711816, readNoise : 4.717724, saturation : 133621.531250 } |
C02 : { gain : 0.717907, readNoise : 4.945924, saturation : 135765.281250 } |
C01 : { gain : 0.775595, readNoise : 6.067882, saturation : 160979.125000 } |
C00 : { gain : 0.725844, readNoise : 5.204304, saturation : 135903.109375 } |
They are being flagged as saturated, because the gain is listed as ~0.7 (too low, it is actually more like 1.1). For C06, 134819*0.715=96395, so anything above this is flagged as saturated. For C01, 160979*0.775=131036, so this is why C01 is not flagged as saturated. So we need to do two things:
(1) This will happen even with the right values, so we need to leave the test for too few pixels in ptc.py. this is the line at: https://github.com/lsst/cp_pipe/blob/28eb519a8f97a5f2db6c21412e9172b7e0c0f463/python/lsst/cp/pipe/ptc.py#L683, but we need to increase the test to np.sum(w) < 10000 , or some reasonable number.
(2) We need to update the gain, noise, and sat level values in the Rxx.yaml files. i am willing to work to get the right values in there.
Attachments
Activity
Thanks Merlin. An excellent point. Let me try re-doing the ISR without interpolation and see if that fixes the problem.
Setting doSaturation=False in the ISR seems to have fixed this. The same CCD and expId as above now gives the number of good pixels by amp below. I'll run some more tests, but this is clearly the right approach.
C10 974426 |
C11 974446 |
C12 974814 |
C13 974982 |
C14 974602 |
C15 974891 |
C16 974624 |
C17 974101 |
C07 974814 |
C06 974331 |
C05 974850 |
C04 974716 |
C03 974722 |
C02 974675 |
C01 974784 |
C00 974564 |
That's great news. I can't see how Christopher Waters or Andrés Alejandro Plazas Malagón would object to that being the default either, so if they confirm, we'll get it switched over so you don't have to add that.
I don't know if we can switch it over. With the refactoring, the ISR task is now separate from the ptc task. So the ISR task has no way of knowing that you are doing the ISR in preparation for a ptc task. So I think you have to specify it.
| Field | Original Value | New Value |
|---|---|---|
| Epic Link |
|
I no longer know how to do it in Gen3, but I'm >95% sure that's not correct.
I think if you have pre-existing postISRCCDs available from running ISR yourself, such that the Quantum Graph generator skips that step, then yes, you're likely correct.
However, if running the task without the pre-existing postISRCCDs available, the graph generator will run ISR itself, and in that mode, should be able to apply whatever defaults we specify.
Christopher Waters could you just confirm/deny whether my rudimentary understanding of Gen3 is correct please?
Well, I'm running it with gen2, so maybe I'm wasting my time.
In Gen2 it's easy - give me 30s...
OK, I'm wrong - in Gen2 in this task it's impossible as you say, but that's just because we're half way through the refactor for Gen3, so this task no longer is capable of running its own isr. This is one of the downsides of being in limbo, sorry.
Point remains though (assuming I'm right above, which I think I have to be, otherwise there are huge problems in Gen3) that this task's "I'm going to run ISR for myself" config needs to turn that off.
The gen3 pipeline defines the ISR stage as well as the PTC stage (as gen3 pipelines are expected to be something like an "end-to-end" process). For development, both gen2 and gen3 should allow you to run ISR once, and then iterate PTC calls as many times as needed on the same set of postISRCCDs. There should be a `cp_pipe/config/ptcIsr.py` that can be passed to `runIsr.py` to generate the postISRCCD files in the way expected by the PTC code.
Christopher Waters That file (ptcIsr.py), along with the corresponding file in cp_pipe/pipelines, is in DM-23159, but not in master: https://github.com/lsst/cp_pipe/blob/tickets/DM-23159/config/ptcIsr.py. In both files, doSaturationInterpolation is already False.
For this ticket, don't we need to set that parameter to false in the ISR config options? https://github.com/lsst/ip_isr/blob/master/python/lsst/ip/isr/isrTask.py#L749
According to the documentation, it is more important to have doSaturation=False, than doSaturationInterpolation=False. If doSaturation=False, then no pixels are flagged as saturated, so there is nothing to interpolate, so it doesn't matter if doSaturationInterpolation is True or False. However, if doSaturation=True and do SaturationInterpolation=False, the pixels will still be flagged as saturated, and so they will still be considered Bad, since they will have a mask value of 2 and not 0.
Can the config file be taken out of DM-23159 and put on this ticket (with the addition of the `doSaturation=False` update)? This avoids the problem of poorly defined saturation values here, and I think (please confirm!) that in the case of data that is truly saturated, simply adds additional points beyond the PTC turnoff that will be excluded anyway.
For the record, here are the ISR configuration options I have been running. Let me know if there are others that should be changed. I think ultimately we will want doCrosstalk=True, but at this point I don't think we have good enough crosstalk coefficients to do that, and it is probably a small correction anyway.
isr.doBias=True isr.doDark=True isr.doFringe=False isr.doSuspect=True isr.doDefect=True isr.overscan.fitType=MEDIAN_PER_ROW isr.overscan.order=1 isr.doFlat=False isr.doSaturation=False isr.doCrosstalk=False isr.edgeMaskLevel=AMP isr.numEdgeSuspect=10 |
| Assignee | Andrés Alejandro Plazas Malagón [ plazas ] |
| Status | To Do [ 10001 ] | In Progress [ 3 ] |
I agree that doCrosstalk should be True. We can leave it like True as default, and set it explicitly to False when calling the ISR.
I'm not sure if we need isr.doFringe=False isr.doSuspect=True in the file. Christopher Waters?
These are the values that are in ptcIsr.py:
|
|
# ISR for inputs of Photon Transfer Curve task
|
config.isr.doWrite = True
|
config.isr.doOverscan = True
|
config.isr.doAssembleCcd = True
|
config.isr.doBias = True
|
config.isr.doVariance = True
|
config.isr.doLinearize = True
|
config.isr.doCrosstalk = True
|
config.isr.doBrighterFatter = False
|
config.isr.doDark = True
|
config.isr.doStrayLight = False
|
config.isr.doFlat = False
|
config.isr.doFringe = False
|
config.isr.doApplyGains = False
|
config.isr.doDefect = True
|
config.isr.doNanMasking: True
|
config.isr.doInterpolate: True
|
config.isr.doSaturation=False
|
config.isr.doSaturationInterpolation = False
|
config.isr.growSaturationFootprintSize = 0 # We want the saturation spillover: it's good signal.
|
My understanding was that we need doSuspect=True in order to have the edge masking. I think the edge masking is important because the flux rolls off significantly near the edges. I thought doSuspect=True, isr.edgeMaskLevel=AMP isr.numEdgeSuspect=10 were all necessary to make the edge masking happen. Maybe I'm wrong.
`doLinearize` can't be True if we're using the PTC dataset to remeasure the linearity. That's probably more of a linearity issue than PTC, however, so leaving it true here makes sense.
And I think `doDefect` enables the edge masking if the `numEdgeSuspect` configuration option is non-zero.
| Reviewers | Andrés Alejandro Plazas Malagón, Christopher Waters [ plazas, cwaters ] | Merlin Fisher-Levine [ mfisherlevine ] |
| Status | In Progress [ 3 ] | In Review [ 10004 ] |
Some minor comments on the config and that's it.
| Status | In Review [ 10004 ] | Reviewed [ 10101 ] |
| Story Points | 1 |
| Resolution | Done [ 10000 ] | |
| Status | Reviewed [ 10101 ] | Done [ 10002 ] |
| Team | Data Release Production [ 10301 ] |
| Attachment | Gain_Differences_GT_5Pct_12673G_24Nov20.pdf [ 46439 ] |
| Attachment | Gain_Differences_GT_5Pct_12673G_24Nov20.pdf [ 46439 ] |
In addition to the above (which are very necessary in their own right), I'd argue that the right fix for this case is almost certainly changing the ISR defaults for the PTC task to not do any interpolation. We fully expect this to be happening on PTC curves, and need to be able to make measurements of where it's actually happening.
Without that change in defaults, we'll only ever be able to measure lower and lower saturation values...