DM-51599: Run crosstalk on CBP data / update and modernize CT code by czwa · Pull Request #355 · lsst/cp_pipe

czwa · 2026-01-14T22:22:36Z

No description provided.

erykoff

I'm wondering if the units are all necessary, but I do see there are contracts to keep things in sync. I would think it could be inferred from the data though?

erykoff · 2026-01-20T23:16:14Z

+  cpCrosstalkExtract:
+    class: lsst.cp.pipe.CrosstalkExtractTask
+    config:
+      growMaskRadius: 20


Should these just be in the parent cpCrosstalkLSST.yaml? Or these are so specific for the LSSTCam analysis?

I measured growMaskRadius only on LSSTCam, and I suspect the value for LATISS would be different (but I haven't compared PSF wings between the two).

erykoff · 2026-01-20T23:17:18Z

+            if np.sum(mask) == 0:
+                continue
+
+            newMask = np.where(np.bitwise_and(mask, detected), maskBit, 0)


This can be mask & detected which I find easier to read than np.bitwise_and().

erykoff · 2026-01-20T23:18:37Z

            outputFluxes=ddict2dict(outputFluxes)
        )

+    def _flipMask(self, maskArray, sourceAmp, targetAmp):


Don't we already have this code in ip_isr? Does it need to be rewritten, or can it be reused?

The ip_isr version expects afw objects, and this uses numpy. It was different enough that I thought the copy-rewrite was worth it.

Can you rename this flipMaskArray then?

erykoff · 2026-01-20T23:19:03Z

-        if config.fluxOrder == 0:
-            self.inputs.discard("inputFluxes")
+        # if config.fluxOrder == 0 and False:
+        #    self.inputs.discard("inputFluxes")


If these shouldn't be here, please remove.

erykoff · 2026-01-20T23:23:28Z

+                    myfluxes = np.ones_like(values)
+
+                if len(values) != len(myfluxes):
+                    self.log.warning(f"Flux and ratio length disagree after first filter: {len(values)} {len(myfluxes)}")  # noqa E501


self.log.warning( f"Flux and ratio length disagree after first filter: {len(values)} {len(myfluxes)}", )

erykoff · 2026-01-20T23:23:40Z

                    values = values[good]
+                    myfluxes = myfluxes[good]
+                if len(values) != len(myfluxes):
+                    self.log.warning(f"Flux and ratio length disagree after second filter: {len(values)} {len(myfluxes)}")  # noqa E501


erykoff · 2026-01-20T23:24:59Z

+                # filter those.
+                pass
+
+        itl_c0 = np.array(itl_c0)


All np.asarray()

erykoff · 2026-01-20T23:25:55Z

+
+        Parameters
+        ----------
+        matrix0 : `np.array`, (Ndet, Namp, Namp)


These are all np.ndarray

erykoff · 2026-02-25T20:41:04Z

    )
+    growMaskRadius = Field(
+        dtype=int,
+        default=0,


I know you said above that you haven't measured this on anything but LSSTCam with CBP, but I still think having this as something non-zero as the default would make sense. Just set this to 20 here and delete the other overrides and we'll figure it out in the future if we need to?

erykoff · 2026-02-25T20:42:24Z

            outputFluxes=ddict2dict(outputFluxes)
        )

+    def _flipMask(self, maskArray, sourceAmp, targetAmp):


Can you rename this flipMaskArray then?

erykoff · 2026-02-25T20:44:24Z

        dtype=int,
        default=0,
-        doc="Polynomial order in source flux to fit crosstalk.",
+        doc="Order of source flux fit to crosstalk. 0=simple linear; 1=first order non-linear.",


I'm getting confused as to why this isn't 1 and 2. What am I missing?

This is the order of the fit done to ratio(source_flux) = target / source_flux. This removes one factor of the source_flux, so a 0-order fit is just a constant crosstalk ratio, and a 1-order fit is a linear fit of an "average" crosstalk ratio with a first-order source_flux correction. I agree the nomenclature is confusing.

erykoff · 2026-02-25T20:46:31Z

-                values = np.array(ratios[ordering[tt]][ordering[ss]])
-                values = values[np.abs(values) < 1.0]  # Discard unreasonable values
+                values = np.asarray(ratios[ordering[tt]][ordering[ss]])
+                good_values = np.abs(values) < 1.0  # Discard unreasonable values


What makes this unreasonable?

This check is designed to remove ratios that have a target location with an actual star there. This translates to excluding all measured ratios that have absolute value greater than 1, such that target_flux > source_flux.

Can you add a comment this in the code?

erykoff

A small comment, otherwise looks good. I also looked and I think that the filtering works even when limited to linear xtalk (e.g. it's looking for bad outliers with < and > and that should work).

erykoff · 2026-03-17T20:06:48Z

-                values = np.array(ratios[ordering[tt]][ordering[ss]])
-                values = values[np.abs(values) < 1.0]  # Discard unreasonable values
+                values = np.asarray(ratios[ordering[tt]][ordering[ss]])
+                good_values = np.abs(values) < 1.0  # Discard unreasonable values


Can you add a comment this in the code?

a) check that flux and ratio length match b) pass fluxes to measurement code, to allow for preliminary non-linear fitting c) log clarifications d) Add stage subsets.

czwa requested a review from erykoff January 14, 2026 22:23

erykoff reviewed Jan 20, 2026

View reviewed changes

czwa force-pushed the tickets/DM-51599 branch from fe6697e to ce85add Compare January 23, 2026 20:42

erykoff reviewed Feb 25, 2026

View reviewed changes

czwa force-pushed the tickets/DM-51599 branch 3 times, most recently from f61fbf3 to ba09639 Compare March 16, 2026 22:27

erykoff approved these changes Mar 17, 2026

View reviewed changes

czwa added 8 commits April 7, 2026 14:42

Add crosstalk pipeline definition.

b7a42ba

Improvements for crosstalk:

a777d4b

a) check that flux and ratio length match b) pass fluxes to measurement code, to allow for preliminary non-linear fitting c) log clarifications d) Add stage subsets.

Add crosstalk filtering step.

cb4a7e5

Add code to set the crosstalk units correctly.

c3eddb5

Fix unit test error and underlying length bug.

c98d0dd

Cleanup and docstring fixes.

a6185dd

Clarify names and comments.

0e9c156

Clarify comment.

2a06b40

czwa force-pushed the tickets/DM-51599 branch from ddefbf2 to 2a06b40 Compare April 8, 2026 21:53

czwa merged commit c65f820 into main Apr 10, 2026
4 checks passed

czwa deleted the tickets/DM-51599 branch April 10, 2026 19:43

Conversation

czwa commented Jan 14, 2026

Uh oh!

erykoff left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

erykoff left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants