DM-37554: Check that ISR usage of gains is consistent #282
Conversation
python/lsst/ip/isr/isrTask.py
Outdated
| Parameters | ||
| ------ | ||
| ptcDataset : `lsst.ip.isr.PhotonTransferCurveDataset` | ||
| Inout Photon Transfer Curve dataset. |
python/lsst/ip/isr/isrTask.py
Outdated
| # Try first with the PTC gains. | ||
| gainProvenanceString = "amp" | ||
| if self.config.usePtcGains: | ||
| if ptcDataset is None: |
There was a problem hiding this comment.
Can you move this check to ~L1348, with the other checks for if doX and X is None? If the config doesn't match what was supplied, it's better to raise early.
python/lsst/ip/isr/isrTask.py
Outdated
| else: | ||
| # Try then with the amplifier gain. | ||
| # We already have a detector at this point. If there was no | ||
| # detector to beging with, one would have been created with |
python/lsst/ip/isr/isrTask.py
Outdated
| noise = overscanResults.residualSigma[0] | ||
| elif self.config.usePtcReadNoise: | ||
| # Try then with the PTC noise. | ||
| if ptcDataset is None: |
There was a problem hiding this comment.
With my suggestion above, I think this just goes away.
python/lsst/ip/isr/isrTask.py
Outdated
| else: | ||
| # Finally, try with the amplifier noise. | ||
| # We already have a detector at this point. If there | ||
| # was no detector to beging with, one would have |
python/lsst/ip/isr/isrTask.py
Outdated
| # self.config.noise. | ||
| noise = amp.getReadNoise() | ||
|
|
||
| self.log.info("%s - Noise provenance: %s, Gain provenance: %s", |
There was a problem hiding this comment.
I think this (and the metadata entries) can be pulled out of the loop. All amps should follow the same logic, so all amps should have the same provenance, right?
python/lsst/ip/isr/isrTask.py
Outdated
|
|
||
| # At this point, the effective PTC should have | ||
| # gain and noise values. | ||
| gain = ptcDataset.gain[ampName] | ||
| if math.isnan(gain): |
There was a problem hiding this comment.
Can this be moved to the effectivePtc method? It's here because this was historically the first use, but keeping all of the logic in one place is simpler.
python/lsst/ip/isr/isrTask.py
Outdated
|
|
||
| metadata = ampExposure.getMetadata() | ||
| metadata[f'LSST GAIN {amp.getName()}'] = gain | ||
| metadata[f'LSST READNOISE {amp.getName()}'] = readNoise | ||
| metadata[f"LSST GAIN {amp.getName()}"] = gain |
There was a problem hiding this comment.
Same with these metadata updates. It's only here for historical reasons.
python/lsst/ip/isr/isrTask.py
Outdated
|
|
||
| isrFunctions.updateVariance( | ||
| maskedImage=ampExposure.getMaskedImage(), | ||
| gain=gain, | ||
| readNoise=readNoise, | ||
| ) | ||
|
|
||
| # isrFunctions.updateVariance applied the gain. | ||
| # Update the units. | ||
| metadata["LSST ISR UNITS"] = 'electrons' |
There was a problem hiding this comment.
I don't think this is true. The image units didn't change.
plazas
force-pushed
the
tickets/DM-37554
branch
from
79eb980
to
3d10c46
Compare
| @@ -774,3 +775,33 @@ def getGoodPoints(self, ampName): | |||
| Boolean array of good points used in PTC. | |||
| """ | |||
| return self.expIdMask[ampName] | |||
|
|
|||
| def validateGainNoiseTurnoffValues(self, ampName): | |||
| """Ensure the gain, read noise, and PTC turnoff have | |||
There was a problem hiding this comment.
Docstring, even though it's trivial.
No description provided.