Skip to content

DM-54870: Fix the buffer bug#222

Merged
arunkannawadi merged 3 commits into
mainfrom
tickets/DM-54870
May 14, 2026
Merged

DM-54870: Fix the buffer bug#222
arunkannawadi merged 3 commits into
mainfrom
tickets/DM-54870

Conversation

@arunkannawadi
Copy link
Copy Markdown
Member

No description provided.

@arunkannawadi arunkannawadi marked this pull request as ready for review May 12, 2026 18:42
Comment thread python/lsst/drp/tasks/metadetection_shear.py Outdated
@arunkannawadi arunkannawadi requested a review from TallJimbo May 12, 2026 19:28
raise InvalidQuantumError("MetadetectionShearTask requires a cell-based skymap.")

cell_config = skymap_config.tractBuilder.active
if (self.config.border is None and cell_config.cellBorder == 0) or (
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is allowing self.config.border == 0 and cell_config.cellBorder == 0 intentional here?

This also makes me wonder if allowing self.config.border=None is useful here; doesn't seem to be materially different from zero (except in this check).

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Allowing self.config.border to be None clarifies that the user intended to rely on the cellBorder defined in the skymap to create the buffer region, and not the border value put in by hand. Allowing self.config.border == 1 and cell_config.cellBorder == 0 would be just as bad as self.config.border == 0 and cell_config.cellBorder == 0. The minimum allowed buffer region would depend on many factors (PSF size, artificial metacal shear applied, cell size etc.) and we are not mandating that specified border is larger than that minimum value.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, I see your point now. The case I absolutely want to avoid is the one where two (different) positive values are set for cellBorder and self.config.border. As long as one of them is a positive integer and the other is not, that is a valid configuration to accept.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

border cannot be set to None.

Comment thread python/lsst/drp/tasks/metadetection_shear.py
Comment thread python/lsst/drp/tasks/metadetection_shear.py Outdated
Comment thread python/lsst/drp/tasks/metadetection_shear.py Outdated
with a quick, hardcoded fix that works for lsst_cells_v2 skymap.
so it can be continued to run on both lsst_cells_v1 and lsst_cells_v2
and any other skymap that we may construct in the future.
@arunkannawadi arunkannawadi merged commit f8a980a into main May 14, 2026
8 checks passed
@arunkannawadi arunkannawadi deleted the tickets/DM-54870 branch May 14, 2026 00:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants