DM-17028: Implement MakeWarp as PipelineTask #263
Conversation
| storageClass="FitsCatalogStorage", | ||
| dimensions=("Visit", "Detector, Tract") | ||
| ) | ||
| jointcal_photoCalib = pipeBase.InputDatasetField( |
There was a problem hiding this comment.
I don't think I'd recommend using underscores in these names - DatasetType name conventions aren't well-defined, but I don't think we want to assume they're the same as coding name conventions (and they de facto aren't right now).
I also don't think we want to prefix these with "jointcal" anyway - once calexp.photoCalib is a thing, I'm thinking we could just do:
config.wcs = "calexp.wcs"
config.photoCalib = "calexp.photoCalib"
instead of
config.wcs = "jointcal_wcs"
config.photoCalib = "jointcal_photoCalib"
without any custom code for either path (or "fgcm_photoCalib", for that matter). That might involve getting some where's-the-Jacobian logic in better shape, too, but I think it's something to aim towards.
I'd be curious to figure out why preflight is angry about the tract, but that's not a high priority if you're not using this anyway. As a side note on that, we should figure out a convention on whether or not to explicitly include dimensions like "Instrument" and "SkyMap" (which are automatically included if you say something that depends on them, like "Visit" or "Tract"). I don't actually recall what @natelust and @czwa have been doing on that front.
There was a problem hiding this comment.
Since these datatypes were a placeholder anyway, I'm dropping them and punting until DM-17062. Outstanding problem the check that doApplyUberCal is False is in validate (the normal place to check). Pipelinetask validating and freezing configs is high on my priority list, as we talked about yesterday.
| """Construct warps for requested warp type for single epoch | ||
|
|
||
| PipelineTask (Gen3) entry point to warp and optionally PSF-match | ||
| calexpes. This method is analogous to `runDataRef`, it prepares all |
| """ | ||
| # Construct skyInfo expected by `run` | ||
| skyMap = inputData["skyMap"] | ||
| outputDataId = next(iter(outputDataIds.values())) |
There was a problem hiding this comment.
If (as it seems) this is guaranteed to be a single-element dict, my favorite pattern for this is:
outputDataId, = outputDataIds.values()
There was a problem hiding this comment.
It's not a single-element dict. It returns two data products directWarps and psfMatchedWarps
| inputData['dataIdList'] = dataIdList | ||
|
|
||
| # Construct list of ccdIds expected by `run` | ||
| inputData['ccdIdList'] = [dataId['detector'] for dataId in dataIdList] |
There was a problem hiding this comment.
These CCD IDs are not the same as the ones currently being produced by MakeCoaddTempExp.runDataRef - those are ccdExposureIds that have the visit mangled in. If you want those here, you can do:
inputData["ccdIdList"] = [butler.registry.packDataId("VisitDetector", dataId) for dataId in dataIdList]
I am also happy to do this on DM-15843, in which I'm already doing similar things to existing PipelineTasks to pass the time between reviews (it's definitely lower-priority than this ticket).
There was a problem hiding this comment.
Your recipe perfectly replicates the old ccdExposureIds. What's the "this" that you're offering?
| mi += background.getImage() | ||
| if self.config.doApplySkyCorr: | ||
| mi -= skyCorr.getImage() | ||
| del mi |
There was a problem hiding this comment.
I assume it's just copied from somewhere else, but I'm pretty confident this line is totally unnecessary; reassigning the variable or ending the function will also make it available for garbage collection.
| dimensions=("Visit", "Detector") | ||
| ) | ||
| skyCorrList = pipeBase.InputDatasetField( | ||
| doc="SkyCorr", |
There was a problem hiding this comment.
We should remember to go back and expand these docstrings later if we're not doing them now. If you don't have a pre-existing list for such things (ticket or otherwise) I can start one.
There was a problem hiding this comment.
I had entirely intended to go back and write complete docstrings for these on this ticket.
yalsayyad
force-pushed
the
tickets/DM-17028
branch
2 times, most recently
from
7c8a616
to
a9e5c7d
Compare
* Both MakeWarpTask and MakeCoaddTempExpTask warp and optionally PSF-match
* They share a `run` method which operates on a single visit
* MakeCoaddTempExp has a runDataRef that operates on a list of visits,
looping over them and calling `run`. Unchanged in this ticket.
* New MakeWarp's `adaptArgsAndRun` method operates on a single visit,
and PipelineTask coordinates parallelizing over multiple visits.
yalsayyad
force-pushed
the
tickets/DM-17028
branch
from
a9e5c7d
to
419087a
Compare
No description provided.