Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

DM-45976: In CI mode, only raise if there are zero parents to deblend. #101

Merged
merged 1 commit into from
Sep 12, 2024
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 7 additions & 5 deletions python/lsst/meas/deblender/sourceDeblendTask.py
Original file line number Diff line number Diff line change
Expand Up @@ -307,15 +307,17 @@ def deblend(self, exposure, srcs, psf):
minChildren, maxChildren = self.config.ciDeblendChildRange
nPeaks = np.array([len(src.getFootprint().peaks) for src in srcs])
childrenInRange = np.where((nPeaks >= minChildren) & (nPeaks <= maxChildren))[0]
if len(childrenInRange) < self.config.ciNumParentsToDeblend:
raise ValueError("Fewer than ciNumParentsToDeblend children were contained in the range "
"indicated by ciDeblendChildRange. Adjust this range to include more "
"parents.")
numParentsToDeblend = self.config.ciNumParentsToDeblend
if len(childrenInRange) < numParentsToDeblend:
# Only use the top N instead of the full requested list.
if len(childrenInRange) == 0:
raise ValueError("No children to deblend; cannot continue with CI testing.")
Copy link
Contributor

Choose a reason for hiding this comment

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

This is fine but I'm also fine with taking Dan's full suggestion that this isn't the place to check for whether or not there are any blends. So you could just warn here and proceed with no sources to deblend, since there could be other visits (in other bands) that are deblended. Nothing says that there have to be any blends in a visit.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have no idea how to do that, and really need to get this out the door. I think this is better than what was there before.

Copy link
Contributor

Choose a reason for hiding this comment

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

Sorry if I wasn't clear, I meant instead of raising here I would just issue a warning.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Even that has some challenges because I'm not sure how to fix downstream in this task if there really were zero. I think this would be impossible anyway (call the deblender with no sources or something?)

Copy link
Contributor

Choose a reason for hiding this comment

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

Ok. Like I said, this is fine and if you want to push forward with it you can. But if we come up against instances where even this constraint is too stringent then we can make it a warning later.

numParentsToDeblend = len(childrenInRange)
# Keep all of the isolated parents and the first
# `ciNumParentsToDeblend` children
parents = nPeaks == 1
children = np.zeros((len(srcs),), dtype=bool)
children[childrenInRange[:self.config.ciNumParentsToDeblend]] = True
children[childrenInRange[:numParentsToDeblend]] = True
srcs = srcs[parents | children]
# We need to update the IdFactory, otherwise the the source ids
# will not be sequential
Expand Down
Loading