-
Notifications
You must be signed in to change notification settings - Fork 9
DM-49378: Trim totalBox to destination warped bbox in getTemplate #388
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
Merged
Merged
Changes from all commits
Commits
Show all changes
4 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -27,6 +27,7 @@ | |
| import lsst.geom as geom | ||
| import lsst.afw.geom as afwGeom | ||
| import lsst.afw.table as afwTable | ||
| from lsst.afw.math._warper import computeWarpedBBox | ||
| import lsst.afw.math as afwMath | ||
| import lsst.pex.config as pexConfig | ||
| import lsst.pipe.base as pipeBase | ||
|
|
@@ -277,25 +278,39 @@ def run(self, *, coaddExposureHandles, bbox, wcs, dataIds, physical_filter): | |
| for tract in coaddExposureHandles: | ||
| maskedImages, catalog, totalBox = self._makeExposureCatalog(coaddExposureHandles[tract], | ||
| dataIds[tract]) | ||
| warpedBox = computeWarpedBBox(catalog[0].wcs, bbox, wcs) | ||
| warpedBox.grow(5) # to ensure we catch all relevant input pixels | ||
| # Combine images from individual patches together. | ||
| unwarped = self._merge(maskedImages, totalBox, catalog[0].wcs) | ||
| unwarped, count, included = self._merge(maskedImages, warpedBox, catalog[0].wcs) | ||
| # Delete `maskedImages` after combining into one large image to reduce peak memory use | ||
| del maskedImages | ||
| potentialInput = self.warper.warpExposure(wcs, unwarped, destBBox=bbox) | ||
| if count == 0: | ||
| self.log.info("No valid pixels from coadd patches in tract %s; not including in output.", | ||
| tract) | ||
| continue | ||
| warpedBox.clip(totalBox) | ||
| potentialInput = self.warper.warpExposure(wcs, unwarped.subset(warpedBox), destBBox=bbox) | ||
|
|
||
| # Delete the single large `unwarped` image after warping to reduce peak memory use | ||
| del unwarped | ||
| if not np.any(np.isfinite(potentialInput.image.array)): | ||
| self.log.info("No overlap from coadds in tract %s; not including in output.", tract) | ||
| if np.all(potentialInput.mask.array & potentialInput.mask.getPlaneBitMask("NO_DATA")): | ||
| self.log.info("No overlap from coadd patches in tract %s; not including in output.", tract) | ||
| continue | ||
|
|
||
| catalogs.append(catalog) | ||
| warped[tract] = potentialInput | ||
| warped[tract].setWcs(wcs) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Was this line unnecessary, because it's set in
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, the merged result has the wcs when it is created. |
||
| # Trim the exposure catalog to just the patches that were used. | ||
| tempCatalog = afwTable.ExposureCatalog(self.schema) | ||
| tempCatalog.reserve(len(included)) | ||
| for i in included: | ||
| tempCatalog.append(catalog[i]) | ||
| catalogs.append(tempCatalog) | ||
| warped[tract] = potentialInput.maskedImage | ||
|
|
||
| if len(warped) == 0: | ||
| raise pipeBase.NoWorkFound("No patches found to overlap science exposure.") | ||
| template = self._merge([x.maskedImage for x in warped.values()], bbox, wcs) | ||
| # At this point, all entries will be valid, so we can ignore included. | ||
| template, count, _ = self._merge(warped, bbox, wcs) | ||
| if count == 0: | ||
| raise pipeBase.NoWorkFound("No valid pixels in warped template.") | ||
|
|
||
| # Make a single catalog containing all the inputs that were accepted. | ||
| catalog = afwTable.ExposureCatalog(self.schema) | ||
|
|
@@ -362,7 +377,7 @@ def _makeExposureCatalog(self, exposureRefs, dataIds): | |
|
|
||
| Returns | ||
| ------- | ||
| images : `list` [`lsst.afw.image.MaskedImage`] | ||
| images : `dict` [`lsst.afw.image.MaskedImage`] | ||
| MaskedImages of each of the input exposures, for warping. | ||
| catalog : `lsst.afw.table.ExposureCatalog` | ||
| Catalog of metadata for each exposure | ||
|
|
@@ -372,11 +387,11 @@ def _makeExposureCatalog(self, exposureRefs, dataIds): | |
| catalog = afwTable.ExposureCatalog(self.schema) | ||
| catalog.reserve(len(exposureRefs)) | ||
| exposures = (exposureRef.get() for exposureRef in exposureRefs) | ||
| images = [] | ||
| images = {} | ||
| totalBox = geom.Box2I() | ||
|
|
||
| for coadd, dataId in zip(exposures, dataIds): | ||
| images.append(coadd.maskedImage) | ||
| images[dataId] = coadd.maskedImage | ||
| bbox = coadd.getBBox() | ||
| totalBox = totalBox.expandedTo(bbox) | ||
| record = catalog.addNew() | ||
|
|
@@ -393,15 +408,15 @@ def _makeExposureCatalog(self, exposureRefs, dataIds): | |
|
|
||
| return images, catalog, totalBox | ||
|
|
||
| @staticmethod | ||
| def _merge(maskedImages, bbox, wcs): | ||
| def _merge(self, maskedImages, bbox, wcs): | ||
| """Merge the images that came from one tract into one larger image, | ||
| ignoring NaN pixels and non-finite variance pixels from individual | ||
| exposures. | ||
|
|
||
| Parameters | ||
| ---------- | ||
| maskedImages : `list` [`lsst.afw.image.MaskedImage`] | ||
| maskedImages : `dict` [`lsst.afw.image.MaskedImage` or | ||
| `lsst.afw.image.Exposure`] | ||
| Images to be merged into one larger bounding box. | ||
| bbox : `lsst.geom.Box2I` | ||
| Bounding box defining the image to merge into. | ||
|
|
@@ -413,10 +428,24 @@ def _merge(maskedImages, bbox, wcs): | |
| merged : `lsst.afw.image.MaskedImage` | ||
| Merged image with all of the inputs at their respective bbox | ||
| positions. | ||
| count : `int` | ||
| Count of the number of good pixels (those with positive weights) | ||
| in the merged image. | ||
| included : `list` [`int`] | ||
| List of indexes of patches that were included in the merged | ||
| result, to be used to trim the exposure catalog. | ||
| """ | ||
| merged = afwImage.ExposureF(bbox, wcs) | ||
| weights = afwImage.ImageF(bbox) | ||
| for maskedImage in maskedImages: | ||
| included = [] # which patches were included in the result | ||
| for i, (dataId, maskedImage) in enumerate(maskedImages.items()): | ||
| # Only merge into the trimmed box, to save memory | ||
| clippedBox = geom.Box2I(maskedImage.getBBox()) | ||
| clippedBox.clip(bbox) | ||
| if clippedBox.area == 0: | ||
| self.log.debug("%s does not overlap template region.", dataId) | ||
| continue # nothing in this image overlaps the output | ||
| maskedImage = maskedImage.subset(clippedBox) | ||
| # Catch both zero-value and NaN variance plane pixels | ||
| good = (maskedImage.variance.array > 0) & (np.isfinite(maskedImage.variance.array)) | ||
| weight = maskedImage.variance.array[good]**(-0.5) | ||
|
|
@@ -432,10 +461,11 @@ def _merge(maskedImages, bbox, wcs): | |
| # `weight` are the exact values we want to scale by. | ||
| maskedImage.image.array[good] *= weight | ||
| maskedImage.variance.array[good] *= weight | ||
| weights[maskedImage.getBBox()].array[good] += weight | ||
| weights[clippedBox].array[good] += weight | ||
| # Free memory before creating new large arrays | ||
| del weight | ||
| merged.maskedImage[maskedImage.getBBox()] += maskedImage | ||
| merged.maskedImage[clippedBox] += maskedImage | ||
| included.append(i) | ||
|
|
||
| good = weights.array > 0 | ||
|
|
||
|
|
@@ -448,7 +478,7 @@ def _merge(maskedImages, bbox, wcs): | |
|
|
||
| merged.mask.array[~good] |= merged.mask.getPlaneBitMask("NO_DATA") | ||
|
|
||
| return merged | ||
| return merged, good.sum(), included | ||
|
|
||
| def _makePsf(self, template, catalog, wcs): | ||
| """Return a PSF containing the PSF at each of the input regions. | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My understanding of
computeWarpedBBoxis that that already errs on the side of a larger box, so growing it here is probably unnecessary. That said, an extra 5 pixels shouldn't make much of a difference, and it will still be fewer pixels than before.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I found that without growing the box by a few pixels, the corners in the tests weren't correct, so I decided to be a bit generous here.