DM-49175: Adjust configs/warnings for LSSTCam dense PTC #301

Alex-Broughton · 2025-03-16T19:37:32Z

No description provided.

erykoff · 2025-03-20T16:54:07Z

python/lsst/cp/pipe/ptc/cpPtcSolve.py

@@ -1211,6 +1211,7 @@ def errFunc(p, x, y):
                        & (np.abs(np.nan_to_num(sigResids)) < sigmaCutPtcOutliers)
                        & mask
                    )
+


Please remove this newline (there are no changes to this file on this PR)

Noting this

erykoff · 2025-03-20T16:54:30Z

python/lsst/cp/pipe/cpCtiSolve.py

+                # There are no exposures left, so set trap so that
+                # there is no correction
+                self.log.warning("All exposures brighter than config.maxImageMean are excluded. "
+                                 "Setting zero-sized serial trap for amp {ampName}.")


Please fix the warning to "amp %s", ampName)

erykoff · 2025-03-20T16:55:01Z

python/lsst/cp/pipe/cpCtiSolve.py

-                raise RuntimeError("All exposures brighter than config.maxSignalForCti and excluded.")
+                # There are no exposures left, set globalCTI to 0
+                self.log.warning("All exposures brighter than config.maxSignalForCti are excluded. "
+                                 f"Setting {ampName} global CTI to zero.")


Please fix warning to "Setting %s global CTI to zero.", ampName)

erykoff · 2025-03-20T16:55:24Z

python/lsst/cp/pipe/cpCtiSolve.py

+                # All exposures excluded, set the calibration so that
+                # there is no correction
+                self.log.warning("All exposures brighter than config.maxImageMean are excluded. "
+                                 "Setting local offset drift scale to zero for amp {ampName}.")


Please fix warning to "...for amp %s.", ampName

This needs to be fixed.

erykoff · 2025-03-20T16:55:57Z

bps/templates/bps_ptc.yaml


 payload:
  payloadName: "${INSTRUMENT}_${TICKET}_ptc"
  output: "${USER_CALIB_PREFIX}${INSTRUMENT}/calib/${TICKET}/${TAG}/ptcGen.${RERUN}"
  butlerConfig: "${REPO}"
-  inCollection: "${USER_CALIB_PREFIX}${INSTRUMENT}/calib/${TICKET},${RAW_COLLECTION},${CALIB_COLLECTIONS}"
+  inCollection: ${USER_CALIB_PREFIX}${INSTRUMENT}/calib/${TICKET}/${TAG},${RAW_COLLECTION},${CALIB_COLLECTIONS}"


You lost a quote here, it will crash. Also this should not have the tag.

erykoff · 2025-03-20T18:42:56Z

bps/templates/bps_ptc.yaml


 payload:
  payloadName: "${INSTRUMENT}_${TICKET}_ptc"
  output: "${USER_CALIB_PREFIX}${INSTRUMENT}/calib/${TICKET}/${TAG}/ptcGen.${RERUN}"
  butlerConfig: "${REPO}"
-  inCollection: "${USER_CALIB_PREFIX}${INSTRUMENT}/calib/${TICKET},${RAW_COLLECTION},${CALIB_COLLECTIONS}"
+  inCollection: ${USER_CALIB_PREFIX}${INSTRUMENT}/calib/${TICKET},${RAW_COLLECTION},${CALIB_COLLECTIONS}"


This is still wrong, it lost a quote.

erykoff · 2025-03-20T18:43:44Z

python/lsst/cp/pipe/cpCtiSolve.py

@@ -312,9 +312,20 @@ def solveLocalOffsets(self, inputMeasurements, calib, detector):
                    data.append(exposureDict[ampName]['SERIAL_OVERSCAN_VALUES'][start:stop+1])
                else:
                    Nskipped += 1
-            self.log.info(f"Skipped {Nskipped} exposures brighter than {self.config.maxImageMean}.")
+            self.log.info("Skipped %d exposures brighter than %f.",
+                          Nskipped, self.config.maxImageMean)


Since you're in here can you fix Nskipped (against the dev guide) to nSkipped?

erykoff · 2025-03-20T18:44:03Z

python/lsst/cp/pipe/cpCtiSolve.py

+                # All exposures excluded, set the calibration so that
+                # there is no correction
+                self.log.warning("All exposures brighter than config.maxImageMean are excluded. "
+                                 "Setting local offset drift scale to zero for amp {ampName}.")


This needs to be fixed.

erykoff · 2025-03-20T18:44:26Z

python/lsst/cp/pipe/cpCtiSolve.py

@@ -697,9 +713,17 @@ def findTraps(self, inputMeasurements, calib, detector):
                    new_signal.append(exposureDict[ampName]['LAST_COLUMN_MEAN'])
                else:
                    Nskipped += 1
-            self.log.info(f"Skipped {Nskipped} exposures brighter than {self.config.maxImageMean}.")
+            self.log.info("Skipped %d exposures brighter than %f.",
+                          Nskipped, self.config.maxImageMean)


Same here for Nskipped

erykoff · 2025-03-20T18:45:21Z

python/lsst/cp/pipe/cpCtiSolve.py

@@ -821,7 +845,8 @@ def calcEper(self, mode, inputMeasurements, amp):
            # Number of parallel shifts = nRows
            nShifts = amp.getRawDataBBox().getHeight()
        else:
-            raise RuntimeError(f"{mode} is not a known orientation for the EPER calculation.")
+            raise RuntimeError("%s is not a known orientation for the EPER "
+                               "calculation.", mode)


Here f-strings are okay, and also this doesn't work with raising, just logs; please revert.

erykoff · 2025-03-20T18:45:50Z

python/lsst/cp/pipe/ptc/cpPtcSolve.py

@@ -1211,6 +1211,7 @@ def errFunc(p, x, y):
                        & (np.abs(np.nan_to_num(sigResids)) < sigmaCutPtcOutliers)
                        & mask
                    )
+


Noting this

erykoff · 2025-03-20T18:46:09Z

bps/templates/bps_bfk.yaml

 project: "${TICKET}"
 campaign: "${TICKET}"
+submitPath: $SCRATCH/submit/{outputRun}


Please protect these (and below) with quotes.

erykoff · 2025-03-20T19:03:36Z

python/lsst/cp/pipe/cpCtiSolve.py

@@ -304,17 +304,29 @@ def solveLocalOffsets(self, inputMeasurements, calib, detector):
            # leaks into the overscan region.
            signal = []
            data = []
-            Nskipped = 0
+            NSkipped = 0


nSkipped. Must start lower case.

erykoff · 2025-03-20T19:48:11Z

Please update the name of the PR to be more descriptive (e.g., add clustering to bps templates and LSSTCam config overrides)

…ut struct

…radient

Alex-Broughton force-pushed the tickets/DM-49175 branch 7 times, most recently from a94acef to 486a7e8 Compare March 20, 2025 16:20

erykoff reviewed Mar 20, 2025

View reviewed changes

Alex-Broughton force-pushed the tickets/DM-49175 branch 2 times, most recently from 7e662f8 to fcfcdb5 Compare March 20, 2025 18:57

erykoff reviewed Mar 20, 2025

View reviewed changes

Alex-Broughton force-pushed the tickets/DM-49175 branch 2 times, most recently from 99a1e83 to 9c063e8 Compare March 20, 2025 19:09

erykoff approved these changes Mar 20, 2025

View reviewed changes

Alex-Broughton changed the title ~~DM-49175: Create LSSTCam calibrations~~ DM-49175: Adjust configs/warnings for LSSTCam dense PTC Mar 20, 2025

Alex-Broughton force-pushed the tickets/DM-49175 branch 3 times, most recently from d883f67 to 94dd0dc Compare March 26, 2025 07:05

Alex-Broughton added 10 commits March 31, 2025 12:43

Add ampwise-averaged combined calibration frame CalibCombineTask outp…

c3e5c01

…ut struct

Change CTI solve errors to warnings if no exposures left after clip.

b7d1204

Change mean combined calib output to a config option

110e5d1

Move submit path to SCRATCH in bps templates

e48d3b5

Add PTC outlier rejection based on maximum residual sigma vs signal g…

5b6b51d

…radient

Lower sigmaCutPtcOutliers

b4f7ae3

Add clustering by exposure for all isr task pipelines

85b7ff5

Fix typo in cpBfkSolve pipeline step name

b8a5d89

Undo typo fix

0466ad9

Add to user collections

8025bb2

Alex-Broughton added 4 commits March 31, 2025 12:43

Adjust configs for LSSTCam dense PTC

d05be02

Change f-strings to %-style

c5ff93a

Remove TAG collection from user calibs

6206bb9

Fix typos move old variable names to proper format

4c6cf56

Alex-Broughton force-pushed the tickets/DM-49175 branch from 94dd0dc to 8e544f6 Compare April 1, 2025 23:45

Add doManualDefects = true to LSSTCam cpDefects pipelines

a2e2d2f

Alex-Broughton force-pushed the tickets/DM-49175 branch from 8e544f6 to a2e2d2f Compare April 1, 2025 23:47

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

DM-49175: Adjust configs/warnings for LSSTCam dense PTC #301

DM-49175: Adjust configs/warnings for LSSTCam dense PTC #301

Alex-Broughton commented Mar 16, 2025

erykoff Mar 20, 2025

erykoff Mar 20, 2025

erykoff Mar 20, 2025

erykoff Mar 20, 2025

erykoff Mar 20, 2025

erykoff Mar 20, 2025

erykoff Mar 20, 2025

erykoff Mar 20, 2025

erykoff Mar 20, 2025

erykoff Mar 20, 2025

erykoff Mar 20, 2025

erykoff Mar 20, 2025

erykoff Mar 20, 2025

erykoff Mar 20, 2025

erykoff Mar 20, 2025

erykoff commented Mar 20, 2025

DM-49175: Adjust configs/warnings for LSSTCam dense PTC #301

Are you sure you want to change the base?

DM-49175: Adjust configs/warnings for LSSTCam dense PTC #301

Conversation

Alex-Broughton commented Mar 16, 2025

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

erykoff commented Mar 20, 2025