DM-33745: Refactor getTemplate pipelineTask #211
Conversation
python/lsst/ip/diffim/getTemplate.py
Outdated
| default=10, | ||
| doc="Number of pixels to grow the requested template image to account for warping" | ||
| ) | ||
| coaddName = pexConfig.Field( |
There was a problem hiding this comment.
You can delete the configs coaddName and warpType now. In Gen3 they are templates in the connection names, not configs.
| pass | ||
|
|
||
|
|
||
| class GetMultiTractCoaddTemplateTask(GetTemplateTask): |
There was a problem hiding this comment.
Would you also do a search the stack for GetMultiTractCoaddTemplateTask and replace our invocations of it too? I know you'll find it in ap_pipe and drp_pipe. Without that, this will just add warnings to our logs.
python/lsst/ip/diffim/getTemplate.py
Outdated
| # SkyMap only needed for filtering without | ||
| inputs.pop('skyMap') | ||
| outputs = self.run(**inputs) | ||
| coaddExposuresRefs, patchList = self.getOverlappingExposures(inputs['coaddExposures'], |
There was a problem hiding this comment.
You free for paircoding tomorrow? I have an idea:
- Have
getOverlappingExposurestake the deferredDatasetHandles, construct the DCR models, and return actualexposures(not deferredDatasetHandles). (6 patches is an acceptable number to have in memory) - Keep
runresponsible for just the warping +CoaddPsfconstruction part.
python/lsst/ip/diffim/getTemplate.py
Outdated
| finalBBox = bbox | ||
| tempBBox = geom.Box2I(bbox.getBegin(), bbox.getDimensions()) |
There was a problem hiding this comment.
Curious why you changed the template to have the bbox of the original calexp bbox rather the grown one.
isullivan
force-pushed
the
tickets/DM-33745
branch
2 times, most recently
from
615a0fe
to
bd644a5
Compare
python/lsst/ip/diffim/getTemplate.py
Outdated
| @@ -582,7 +573,8 @@ def getOverlappingExposures(self, inputs): | |||
| detectorPolygon = geom.Box2D(inputs['bbox']) | |||
| overlappingArea = 0 | |||
| coaddExposureList = [] | |||
| for coaddRef in inputs['coaddExposures']: | |||
| dataIds = [] | |||
| for coaddRef in inputs["coaddExposures"]: | |||
There was a problem hiding this comment.
The only difference on this line is ' to ".
python/lsst/ip/diffim/getTemplate.py
Outdated
| continue | ||
|
|
||
| exp = afwImage.ExposureF(finalBBox, finalWcs) | ||
| exp.maskedImage.set(np.nan, afwImage.Mask.getPlaneBitMask("NO_DATA"), np.nan) | ||
| exp.maskedImage.assign(warped.maskedImage, warped.getBBox()) | ||
| warpedBBox = warped.getBBox() | ||
| warpedBBox.clip(finalBBox) |
There was a problem hiding this comment.
These 2 added lines (and mod one line under) are not necessary: warpExposure clips the destination BBox to the maxBBox=finalBBox.
python/lsst/ip/diffim/getTemplate.py
Outdated
| ConfigClass = GetDcrTemplateConfig | ||
| _DefaultName = "getDcrTemplateTask" | ||
|
|
||
| def __init__(self, *args, **kwargs): |
There was a problem hiding this comment.
This __init__ method does nothing more than its parent
python/lsst/ip/diffim/getTemplate.py
Outdated
|
|
||
| class GetDcrTemplateTask(GetTemplateTask): | ||
| ConfigClass = GetDcrTemplateConfig | ||
| _DefaultName = "getDcrTemplateTask" |
There was a problem hiding this comment.
My fault for including the Task in the original getTemplate Tasks. Should be _DefaultName = "getDcrTemplate"
python/lsst/ip/diffim/getTemplate.py
Outdated
| _DefaultName = "getMultiTractCoaddTemplateTask" | ||
| class GetTemplateTask(pipeBase.PipelineTask): | ||
| ConfigClass = GetTemplateConfig | ||
| _DefaultName = "getTemplateTask" |
There was a problem hiding this comment.
My fault for including the Task in the original getTemplate Tasks' DefaultName. Should be _DefaultName = "getTemplate"
python/lsst/ip/diffim/getTemplate.py
Outdated
| super().__init__(*args, **kwargs) | ||
|
|
||
| def getOverlappingExposures(self, inputs): | ||
| """Return list of coaddExposure DeferredDatasetHandles that overlap detector |
There was a problem hiding this comment.
Agh, we made this summary doc string not true anymore. How about "Return coadds that overlap detector" or if you want a longer one: "Return lists of coadds and their corresponding dataIds that overlap the detector"
for this one and the parent's getOverlappingExposures too
| @@ -644,31 +634,32 @@ def run(self, coaddExposures, bbox, wcs): | |||
| maskedImageList = [] | |||
| weightList = [] | |||
|
|
|||
| for coaddExposure in coaddExposures: | |||
| coaddPatch = coaddExposure.get() | |||
| for coaddExposure, dataId in zip(coaddExposures, dataIds): | |||
There was a problem hiding this comment.
calling it coaddExposure instead of coaddPatch adds 7 extra lines to your PR.
isullivan
force-pushed
the
tickets/DM-33745
branch
from
bd644a5
to
af55a3d
Compare
This PR changes the name of
GetMultiTractCoaddTemplateTaskto be simplyGetTemplateTask, adds support for creating templates from DCR coadds, and adds deprecation warnings to tasks with the old names (which inherit from the new tasks).