DM-23983: Cannot apply crosstalk in Gen 3 DECam processing #150

czwa · 2020-05-18T19:43:36Z

Handle crosstalk via a CrosstalkCalibs persisted as FITS/YAML. Use ip_isr's updated CrosstalkTask for application, even for inter-ccd crosstalk. obs_decam still needs to subclass CrosstalkTask to implement the prepCrosstalk() method with gen2 butlers.

mrawls · 2020-05-29T22:15:10Z

decam/SConscript

+
+makeCrosstalk = env.Command(crosstalkOutDir, crosstalkInFile, command)
+
+# Ingest crosstalk


Suggest a second comment here explaining that this calls ingestCuratedCalibs.py to put crosstalk calib products in the built obs_decam package. Please update the very top comment line of this script to indicate that it does things pertaining to linearity, defects, and crosstalk.

mrawls · 2020-05-29T22:28:54Z

decam/makeCrosstalkDecam.py

+    dataDict['coeffs'] = dataDict['coeffs'].transpose()
+
+    decamCT = ipIsr.crosstalk.CrosstalkCalib.fromDict(dataDict)
+    # Lie to ingestCuratedCalibs, because it interprets calibDate as validStart.


lol... how do defects handle this?

The conversion of existing defects wrote the yaml representation (what this script is attempting to duplicate) to files containing the explicit start points, and set CALIBDATE to that value. The new defects use the midpoint of the input observations (which I don't think it correct, either). I've updated the comment to be more informative, and this will likely be revisited in the future when all the calibrations are more unified.

mrawls · 2020-05-29T22:31:17Z

decam/makeCrosstalkDecam.py

+        Raised if the detector is not known.
+    """
+    ampIndexMap = {'A': 0, 'B': 1}
+    detMap = {'ccd01': 'S29', 'ccd02': 'S30', 'ccd03': 'S31', 'ccd04': 'S25', 'ccd05': 'S26',


This nearly duplicates

obs_decam/python/lsst/obs/decam/decamMapper.py

Lines 49 to 57 in 0afad47

detectorNames = {

1: 'S29', 2: 'S30', 3: 'S31', 4: 'S25', 5: 'S26', 6: 'S27', 7: 'S28', 8: 'S20', 9: 'S21',

10: 'S22', 11: 'S23', 12: 'S24', 13: 'S14', 14: 'S15', 15: 'S16', 16: 'S17', 17: 'S18',

18: 'S19', 19: 'S8', 20: 'S9', 21: 'S10', 22: 'S11', 23: 'S12', 24: 'S13', 25: 'S1', 26: 'S2',

27: 'S3', 28: 'S4', 29: 'S5', 30: 'S6', 31: 'S7', 32: 'N1', 33: 'N2', 34: 'N3', 35: 'N4',

36: 'N5', 37: 'N6', 38: 'N7', 39: 'N8', 40: 'N9', 41: 'N10', 42: 'N11', 43: 'N12', 44: 'N13',

45: 'N14', 46: 'N15', 47: 'N16', 48: 'N17', 49: 'N18', 50: 'N19', 51: 'N20', 52: 'N21',

53: 'N22', 54: 'N23', 55: 'N24', 56: 'N25', 57: 'N26', 58: 'N27', 59: 'N28', 60: 'N29',

62: 'N31'}

, is that because the mapper is going to go away? Is there a more generic place for this mapping to land?

Updated to use DecamMapper.detectorNames here and for detSerialMap (which is just a flip of key:value to value:key). If the mapper does go away, this will suddenly fail, and will need to be handled at that time.
I think the "more generic" place is the obs_decam Instrument definition, but I'm not sure.

mrawls · 2020-05-30T00:51:45Z

decam/makeCrosstalkDecam.py

+    decamCT.writeText(f"{outDir}/1970-01-01T00:00:00.yaml")
+
+
+def readFile(fromFile):


fromFile is an awkward variable name, perhaps crosstalkFile? I do appreciate that "from" indicates it's to be read from and not written to, though, so maybe crosstalkInfile?

fromFile -> crosstalkInfile

mrawls · 2020-05-30T00:52:55Z

pipelines/RunIsr.yaml

@@ -0,0 +1,32 @@
+description: ip_isr handling of DECAM specific full FP CT solution


If permitted with character limits etc., DECAM -> DECam, FP -> focal plane, CT -> crosstalk

mrawls · 2020-05-30T00:56:00Z

policy/DecamMapper.yaml

+    reference: raw_visit
+    storage: FitsStorage
+    tables: crosstalk
+    template: crosstalk/%(calibDate)s/crosstalk-%(calibDate)s-%(ccdnum)03d.fits


It seems weird to me that the date that lands in the file path will be something in 1970 and have no bearing on how recent the crosstalk coefficients actually are. This probably doesn't matter 99% of the time... what do defects do?

It's the same thing as this: calibDate is used to index the files, using 1970 as the "for all data" value, with later dates used to indicate when the previous calibration should be superseded. If the crosstalk changes, we'll either need to replace the xtalk text file that is used to generate the tables (if the new values are better measurements), or will need to modify makeCrosstalkDecam.py to set calibDate differently (if the new values represent a physical change in the crosstalk of the camera).

Fair enough, I guess we will live with it :) validity is so tricky!

mrawls · 2020-05-30T00:58:39Z

python/lsst/obs/decam/crosstalk.py

+            try:
+                source_exposure = butler.get('raw', dataId={'visit': visit, 'ccd': sourceDet})
+            except RuntimeError:
+                self.log.warn('Cannot access source %s, SKIPPING IT' % sourceDet)


Please use f-strings instead of % thingies, unlike whoever wrote the original code 🙃

mrawls · 2020-05-30T01:00:25Z

python/lsst/obs/decam/crosstalk.py

+                for amp in source_exposure.getDetector():
+                    decamisr.overscanCorrection(source_exposure, amp)
+
+                # Do not assemble here, as DECam CT is corrected before assembly.


Is this always the case, now? I remember the old doCrosstalkBeforeAssemble but I don't remember when it was True or False or why.

HSC and DECam have different conventions, and I think there's a linearity correction term that changes the pixel values differently depending on when it's applied.

Fair enough, but there is still a doCrosstalkBeforeAssemble in the ip_isr config. It defaults to False in ip_isr, but unless that config is going away, I would prefer to refer to it here (even if it hardwired one way or the other and there is virtually no reason to have it set differently).

mrawls · 2020-05-30T01:03:03Z

python/lsst/obs/decam/crosstalk.py

+                              (crosstalk._detectorName, sourceDet))
+
+                for amp in source_exposure.getDetector():
+                    decamisr.overscanCorrection(source_exposure, amp)


There was previously a check to see if maybe it had already been overscan corrected, but I suppose that's virtually impossible?

It should be impossible. This only triggers in gen2, and explicitly requests the 'raw'. In gen3, this isn't called because the crosstalkSources there are isrOscanCorr, which always should have the overscan removed.

Gotcha. Incidentally, can the gen3 crosstalkSources be more verbosely isrOverscanCorrected?

mrawls · 2020-05-30T01:04:18Z

python/lsst/obs/decam/crosstalk.py

        return crosstalkSources

-    @pipeBase.timeMethod
-    def run(self, exposure, crosstalkSources=None):


I'm going to assume this all moved to ip_isr and not panic.

It's now inheriting the new CrosstalkTask.run(), and that has all the inter-chip logic now.

ip_isr now supports inter-chip crosstalk, so use that code. Add script to construct compatible crosstalk calibration files.

As part of the transition to the new crosstalk code, Decam now only needs to implement the prepCrosstalk method. This reads the interChip portion of the crosstalk calibration, and uses that to pre-process the other source detectors needed for the current ISR process.

Ignore crosstalk directory.

czwa requested a review from mrawls May 18, 2020 20:10

mrawls reviewed May 30, 2020

View reviewed changes

czwa added 4 commits June 1, 2020 14:46

Use ip_isr.CrosstalkTask().

41f66c2

ip_isr now supports inter-chip crosstalk, so use that code. Add script to construct compatible crosstalk calibration files.

Update mapper and dataDir for new crosstalk.

b0586dd

Ignore crosstalk directory.

Update crosstalk construction and mapping.

43368a5

czwa force-pushed the tickets/DM-23983 branch from 36c1138 to 27998f7 Compare June 1, 2020 20:02

czwa added 3 commits June 9, 2020 18:42

Add RunIsr pipeline.

83fd50f

Clarify comments, docstrings, and log messages.

2723552

Use DecamMapper detectorName dictionary.

563b8a5

czwa force-pushed the tickets/DM-23983 branch from d5361cf to 563b8a5 Compare June 9, 2020 23:48

czwa merged commit 350520e into master Jun 11, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

DM-23983: Cannot apply crosstalk in Gen 3 DECam processing #150

DM-23983: Cannot apply crosstalk in Gen 3 DECam processing #150

czwa commented May 18, 2020

mrawls May 29, 2020

mrawls May 29, 2020

czwa Jun 2, 2020

mrawls May 29, 2020

czwa Jun 2, 2020

mrawls May 30, 2020

czwa Jun 2, 2020

mrawls May 30, 2020

mrawls May 30, 2020

czwa Jun 2, 2020

mrawls Jun 4, 2020

mrawls May 30, 2020

mrawls May 30, 2020

czwa Jun 2, 2020

mrawls Jun 4, 2020

mrawls May 30, 2020

czwa Jun 2, 2020

mrawls Jun 4, 2020

mrawls May 30, 2020

czwa Jun 2, 2020


		makeCrosstalk = env.Command(crosstalkOutDir, crosstalkInFile, command)

		# Ingest crosstalk

	detectorNames = {
	1: 'S29', 2: 'S30', 3: 'S31', 4: 'S25', 5: 'S26', 6: 'S27', 7: 'S28', 8: 'S20', 9: 'S21',
	10: 'S22', 11: 'S23', 12: 'S24', 13: 'S14', 14: 'S15', 15: 'S16', 16: 'S17', 17: 'S18',
	18: 'S19', 19: 'S8', 20: 'S9', 21: 'S10', 22: 'S11', 23: 'S12', 24: 'S13', 25: 'S1', 26: 'S2',
	27: 'S3', 28: 'S4', 29: 'S5', 30: 'S6', 31: 'S7', 32: 'N1', 33: 'N2', 34: 'N3', 35: 'N4',
	36: 'N5', 37: 'N6', 38: 'N7', 39: 'N8', 40: 'N9', 41: 'N10', 42: 'N11', 43: 'N12', 44: 'N13',
	45: 'N14', 46: 'N15', 47: 'N16', 48: 'N17', 49: 'N18', 50: 'N19', 51: 'N20', 52: 'N21',
	53: 'N22', 54: 'N23', 55: 'N24', 56: 'N25', 57: 'N26', 58: 'N27', 59: 'N28', 60: 'N29',
	62: 'N31'}

		decamCT.writeText(f"{outDir}/1970-01-01T00:00:00.yaml")


		def readFile(fromFile):

		@@ -0,0 +1,32 @@
		description: ip_isr handling of DECAM specific full FP CT solution

DM-23983: Cannot apply crosstalk in Gen 3 DECam processing #150

DM-23983: Cannot apply crosstalk in Gen 3 DECam processing #150

Conversation

czwa commented May 18, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment