DM-27598: Many images have few or no good pixels when running ptc.py #66
Conversation
config/ptcIsr.py
Outdated
| config.isr.doNanMasking: True | ||
| config.isr.doInterpolate: True |
There was a problem hiding this comment.
Colons instead of =? Does that even run?
config/ptcIsr.py
Outdated
| config.isr.doDefect = True | ||
| config.isr.doNanMasking: True | ||
| config.isr.doInterpolate: True | ||
| config.isr.doSaturation=False |
There was a problem hiding this comment.
Spaces around operator
config/ptcIsr.py
Outdated
| 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. |
There was a problem hiding this comment.
That comment feels like it's copied and pasted from a crosstalk config.
| config.isr.doAssembleCcd = True | ||
| config.isr.doBias = True | ||
| config.isr.doVariance = True | ||
| config.isr.doLinearize = True |
There was a problem hiding this comment.
I am not sure we want that on, do we? Isn't the most common use-case here that we'll be wanting the uncorrected values, and only turning it back on to check the models we've built, meaning the default should be False? Or is there other thinking here?
There was a problem hiding this comment.
I agree with you; however, Chris points out that more of a linearity issue than PTC and opined that leaving it true here makes sense.
There was a problem hiding this comment.
Does running the linearity generation code have its own config, or does it share this one? I think I really need to catch up on how Gen3 configs and contracts and all that work now 😬
@czwa what are your thoughts on this?
| @@ -0,0 +1,20 @@ | |||
| # ISR for inputs of Photon Transfer Curve task | |||
There was a problem hiding this comment.
Comment on the whole config: what about Craig's suggestion of isr.edgeMaskLevel='AMP'?
There was a problem hiding this comment.
And is the the default the same as his suggestion of isr.numEdgeSuspect=10?
There was a problem hiding this comment.
I'll add these two as defaults.
plazas
force-pushed
the
tickets/DM-27598
branch
from
2b104c5
to
6971aa3
Compare
No description provided.