DM-53312: Robustify the calibrateImage background estimation #1187

laurenam · 2025-11-19T18:49:55Z

No description provided.

erykoff · 2025-11-19T21:55:48Z

python/lsst/pipe/tasks/calibrateImage.py

+        dtype=float,
+        default=3.0,
+        doc="Maximum pedestal allowable in star_background measurement. If the computed value "
+            "exceeds this value, the background pedestal will be set to this value.",


What are the units? Just whatever the post_isr units are?

Yeah, I didn't want to put units that for some reason may be different than our standard processing...

Maybe just quote "in image units" or something like that.

erykoff · 2025-11-19T21:58:09Z

python/lsst/pipe/tasks/calibrateImage.py

        origMask = result.exposure.mask.clone()
        bad_mask_planes = ["BAD", "EDGE", "NO_DATA"]
+        if "PARTLY_VIGNETTED" in result.exposure.mask.getMaskPlaneDict().keys():
+            bad_mask_planes += ["PARTLY_VIGNETTED"]


I'm not sure if I love this being here, baked in. This code always ignores the background in the partly vignetted region? What if we want to use this area in coadds in the future?

It is just ignoring it in the background estimation. Yusra suggested it and it seemed like a good idea since the S/N (and hence the nature of the detections) seems to go haywire in vignetted regions, so best not to trust them in the background measurement? You are the expert here, so if you think it should go, I will remove it.

Ah, since this is a pedestal this does seem reasonable and doesn't preclude using the partly vignetted region in the coadds. The only problem is ... of course ... the corner detectors which are all partly vignetted. These aren't in DRP tests now, but they are processed regularly in RA/NV/etc at least. What happens then?

Any thoughts on this?

I just tried running on detectors 0 and 20. The latter took a hot minute, but they both succeeded...which doesn't seem to make a whole lot of sense, but perhaps there are "just enough" pixels not marked as PARTLY_VIGNETTED. Do you have a strong feeling either way? We have never ignored this plane before, so it may be best not to rock that boat. Also, blinking my before & after plots, the outermost region of the focal plane seems entirely unaffected. Ok...I'm leaning towards dropping it for now....thoughts?

erykoff · 2025-11-19T21:59:38Z

python/lsst/pipe/tasks/calibrateImage.py

+        dtype=int,
+        default=3,
+        doc="Minimum number of footprints in the detection mask for star_background measurement. "
+            "This number will get adjusted to 1 percent of the detected peaks if that number "


1% is always the right value and doesn't need to be configurable?

While I can't guarantee that, I can say that I needed it this high to get to where I needed in many cases, and any higher and it would crap out (either just by imposing an unattainable Goldilocks Zone, or running out of iterations).

That does make me a little nervous ... can you make this configurable in case this needs to be tweaked?

erykoff · 2025-11-19T22:00:18Z

python/lsst/pipe/tasks/calibrateImage.py

-                    starBackgroundDetectionConfig.thresholdValue = 1.2*currentThresh
+                if nFootprintTemp < minFootprints and detected_fraction > 0.9*maxDetFracForFinalBg:
+                    if nFootprintTemp == 1:
+                        starBackgroundDetectionConfig.thresholdValue = 1.4*currentThresh


Where does 1.4 (and 1.2) come from?

Trial and error and a trade-off between "get there faster" and "don't overshoot". The detection plane can be disturbingly sensitive to small tweaks in the threshold, but when there's only a single footprint, a bigger increase is usually needed.

I don't love that but ... sure at the moment...

erykoff · 2025-11-19T22:02:35Z

python/lsst/pipe/tasks/calibrateImage.py

+                             "maximum allowed (%.2f).  Setting it to %.2f",
+                             pedestalBackgroundLevel, self.config.star_background_max_pedestal,
+                             self.config.star_background_max_pedestal)
+            pedestalBackgroundLevel = self.config.star_background_max_pedestal


Are you sure you want to set this to the limit and not zero? In amp offsets, the assumption was "do no harm" and it should be uncorrected assuming something has gone terribly wrong.

It's debatable...if you think zero is better, I'm ok with going that way (again, this hasn't been hitting, so I'll have to check against the full run, which is sure to hit edge cases to make an informed call).

I'll leave this up to you.

Failsafes are now gone...

erykoff · 2025-11-19T22:02:58Z

python/lsst/pipe/tasks/calibrateImage.py

+                             self.config.star_background_max_pedestal)
+            pedestalBackgroundLevel = self.config.star_background_max_pedestal
+
        self.log.warning("Subtracted pedestalBackgroundLevel = %.4f", pedestalBackgroundLevel)


Please change this to an info level log. If this is normal operations, it's not a warning.

erykoff · 2025-11-19T22:04:37Z

python/lsst/pipe/tasks/calibrateImage.py

        pedestalBackground = afwMath.BackgroundList()
        pedestalBackground.append(pedestalBackgroundList[1])
        pedestalBackgroundLevel = pedestalBackground.getImage().array[0, 0]
+        if pedestalBackgroundLevel > self.config.star_background_max_pedestal:


I just realized this is wrong ... this needs to be an absolute value, and the sign preserved if you're setting to the max.

No, it's not symmetric (as we discussed in slack).

Okay, then please add an opposite polarity (as we discussed). And these should probably be set to some fraction of the median background rather than a fixed constant.

This is to have it available as a mask to avoid when measuring the exposure background in calibrateImage.

erykoff · 2025-11-20T01:54:31Z

python/lsst/pipe/tasks/calibrateImage.py

+        doc="Minimum pedestal allowable in star_background measurement, represented here "
+            "as a fraction (the negative of the absolute) of the initial median background "
+            "already subtracted. If the computed value is smaller than this value, the "
+            "background pedestal will be se to this value.",


These are gone (for now, at least...will follow up after the next DRP to see if failsafe's are actually needed).

erykoff · 2025-11-20T01:55:19Z

python/lsst/pipe/tasks/calibrateImage.py

+    star_background_peak_fraction = pexConfig.Field(
+        dtype=float,
+        default=0.01,
+        doc="The minimum number of footprints in the detection mask for star_background measurement. "


This docstring was copied and isn't correct for the config.

erykoff · 2025-11-20T01:56:38Z

python/lsst/pipe/tasks/calibrateImage.py

-        median_background = np.median(backgroundOrig.getImage().array)
-        self.log.warning("Original median_background = %.2f", median_background)
+        median_background_orig = np.median(backgroundOrig.getImage().array)
+        self.log.info("Original median_background = %.2f", median_background_orig)


Thank you for converting to an info on this one! Also ... have we checked at how long this operation takes? In my experience background.getImage() is surprisingly slow. (Not to solve today; but something to consider for the future).

Ok. On the TODO list...

erykoff · 2025-11-20T01:57:01Z

python/lsst/pipe/tasks/calibrateImage.py

        origMask = result.exposure.mask.clone()
        bad_mask_planes = ["BAD", "EDGE", "NO_DATA"]
+        if "PARTLY_VIGNETTED" in result.exposure.mask.getMaskPlaneDict().keys():
+            bad_mask_planes += ["PARTLY_VIGNETTED"]


Any thoughts on this?

erykoff · 2025-11-20T01:58:03Z

python/lsst/pipe/tasks/calibrateImage.py

            self.log.info("nIter = %d, thresh = %.2f: Fraction of pixels marked as DETECTED or "
                          "DETECTED_NEGATIVE in star_background_detection = %.3f "
-                          "(max is %.3f; min is %.3f)",
+                          "(max is %.3f; min is %.3f)  nFooprint = %d (current min is %d)",


nFooprint -> nFootprint.

erykoff · 2025-11-20T02:03:52Z

python/lsst/pipe/tasks/calibrateImage.py

+                             pedestalBackgroundLevel, minPedestal, minPedestal)
+            pedestalBackgroundLevel = minPedestal
+
+        self.log.info("Subtracted pedestalBackgroundLevel = %.4f", pedestalBackgroundLevel)


So ... I'm confused how this even works now. Does this change the value inside the background list?

pedestalBackground = afwMath.BackgroundList() pedestalBackground.append(pedestalBackgroundList[1]) pedestalBackgroundLevel = pedestalBackground.getImage().array[0, 0]

And then ... it's already been subtracted by the task ... so this won't work.

You're so right (and thanks for catching this!). Going with dropping the failsafes for now (will follow-up after the next DRP run).

This includes ensuring a minimum number of footprints in the bespoke background measurement detection task (if there is only a single large contiguous footprint, this indicates that the threshold is too low and will bias the background measurement to unrepresentative values) and adding the SPIKE mask plane to avoid in the background estimation.

erykoff

I guess you decided to leave off the PARTLY_VIGNETTED? I think that makes sense for now; further investigation will be required to know exactly what it's doing for the corners.

Also, please change the title of the PR to describe what is actually happening here -- what and not why.

laurenam requested a review from erykoff November 19, 2025 21:48

erykoff reviewed Nov 19, 2025

View reviewed changes

laurenam force-pushed the tickets/DM-53312 branch 2 times, most recently from 8ec80b7 to ae9cedc Compare November 20, 2025 01:40

laurenam added 2 commits November 19, 2025 17:45

Remove commented out code from testing

8469eb4

Move up setting of diffraction spike mask

cd08d84

This is to have it available as a mask to avoid when measuring the exposure background in calibrateImage.

laurenam force-pushed the tickets/DM-53312 branch from ae9cedc to 9547542 Compare November 20, 2025 01:45

erykoff reviewed Nov 20, 2025

View reviewed changes

laurenam force-pushed the tickets/DM-53312 branch from 9547542 to e7279b0 Compare November 20, 2025 03:10

erykoff approved these changes Nov 20, 2025

View reviewed changes

laurenam changed the title ~~DM-53312: Investigate extreme clipping seen in DM-53195 run~~ DM-53312: Robustify the calibrateImage background estimation Nov 20, 2025

laurenam merged commit b632fb1 into main Nov 20, 2025
3 checks passed

laurenam deleted the tickets/DM-53312 branch November 20, 2025 05:43

DM-53312: Robustify the calibrateImage background estimation #1187

DM-53312: Robustify the calibrateImage background estimation #1187

Uh oh!

Conversation

laurenam commented Nov 19, 2025

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!

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!

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!

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!

Choose a reason for hiding this comment

Uh oh!

erykoff left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development