DM-41956: Exclude diaObjects not contained in the image bounding box.

[isullivan]

No description provided.

[isullivan]

Note: the original PR had the purged diaObjects in loadDiaCatalogs, but I moved that to diaPipe after reviewing DM-41549 and realizing that we will need to eliminate dependence on the science exposure from loadDiaCatalogs in the future.

[ebellm]

The code looks reasonable; can we add a test?

[ebellm]

Looking at this again, I'm wondering if this ticket is what we need after all.

diaCatalogLoader returns diaObjects outside the bbox mainly because we've given it a big pixel buffer--presumably we could set that buffer to zero and skip the purge entirely.

The reason we load with a buffer though is because sometimes sources on the edge of the chip are going to need to match to a DiaObject with a centroid right outside the bbox. If we purge before associating we'll miss that match and artificially create a new diaObject.

What is the problem that is created by the buffer diaObjects? As I trace the logic through diaPipe, they get included in association and passed to diaCalculation and diaForcedSource. But diaCalculation figures out which diaObject records are being updated--so any off-chip diaObjects should just fall through. And in the call to apdb.store, only diaCalResult.updatedDiaObjects are stored, not the full buffered set of diaObjects.

My proposal would be:

• drop the new purge code (sorry! especially since I made you write a test)
• change the butler output to store diaCalResult.updatedDiaObjects rather than the full buffered diaObject set
• perhaps substantially shrink the default pixelMargin used by diaCatalogLoader to limit ourselves to physically plausible off-chip associations. That should keep the speed improvements. However, I note that the default was last changed on DM-30030 by @laurenam who argued that it should match the value of LoadReferenceObjectsConfig.

[isullivan]

I'd like to push back on your proposal, because reducing pixelMargin when loading diaObjects might not be a good solution in the future. The spatial region query to the APDB is only an approximate fit to the shape of a CCD, so we will always have a moderate number of diaObjects outside the chip. Plus, in the future we might move this to preloading before we know the precise pointing, and in that case we will need to include a substantial buffer. You are correct that off-chip diaObjects are handled appropriately in diaCalculation, but the reason we still want to exclude off-chip diaObjects is to save time in association. In my testing, this change sped up diaPipe by a little over 15%, and we need all the speedups we can get!

I'm not very worried about missing associations to diaObjects, since this change only removes diaObjects that are outside of the image bounding box. We aren't able to detect sources or accurately make measurements all the way to the edge of the image, so that gives us a bit of a buffer. If you're worried about losing real associations with diaObjects just outside the image we could grow the bounding box by a few pixels without hurting speed very much.

[ebellm]

Good point about the preload. I do think we need a small buffer, though, or we will get miss-associated DiaSources even if most edge sources are not reliable. Your suggestion to grow the bbox by a small amount (roughly a PSF width or two?) seems reasonable.

[isullivan]

I've added the requested buffer as a config option.