Details
-
Type:
RFC
-
Status: Adopted
-
Resolution: Unresolved
-
Component/s: DM
-
Labels:None
Description
DM-35207 will shortly be adding a task (updateVisitSummary) that produces a new finalVisitSummary dataset that I think can be used to simplify a lot of downstream task and pipeline code.
Like the existing visitSummary dataset, finalVisitSummary is a per-visit ExposureCatalog with a row for each detector, columns with metadata and summary statistics, and object columns that hold various exposure components like the SkyWcs, Psf, ApCorrMap, and PhotoCalib. The new task is intended to be run just before coaddition, and hence after all of those objects are in their final forms - whatever that means for any particular pipeline (e.g. HSC pipelines typically run jointcal and FGCM, while DECam pipelines just use jointcal at present, and ImSim doesn't run either).
As a result, any task that wants the final forms of any of these could just use a single finalVisitSummary connection to obtain any number of them. And that's what I've done on DM-35207 already - since this can be dropped into all of the existing "global" SkyWcs and PhotoCalib connections we have, I've made finalVisitSummary the new default for all of those, and toggled the global/tract config flags to make them the ones used. This already makes the ExposureCatalog outputs that are produced directly by Jointcal, FGCM, and finalizeCharacterization into short-duration temporaries (we can delete them all sometime in step2 in all pipelines I've surveyed). It also makes our pipelines simpler, because they have more common with each other.
This RFC is about going a step further: deprecating existing connections and configurations that select different exposure-component input catalogs in favor of (in most cases) a single finalVisitSummary input. This reduces complexity (good) and flexibility (maybe bad), but I think it's flexibility we don't need (that's the main point I'm interested in requesting comments about). It also has the potential to reduce I/O, which may be significant for some I/O-bound tasks.
Details
MakeWarpTask and CoaddBaseTask
From MakeWarpTask, deprecate all external* connections, the finalizedPsfApCorrCatalog connection, and the wcsList and bboxList connections.
Deprecate all external config fields ; some of these are already deprecated as Gen2, and all of them should have only ever been on MakeWarpTask, not other CoaddBaseTask subclasses.
MakeWarpTask already has a visitSummary connection that can replace all of these, and I believe it does not need to have a config option to make it get exposure components directly from the calexp instead: it is cleaner to run updateVisitSummary in a mode that just passes those objects through (as it currently does in ImSim pipelines).
WriteRecalibratedSourceTableTask
Deprecate all external* connections and related configs.
Add a new visit summary input connection to replace these.
Again, I do not expect to need a config toggle for using the calexp components; we can always pass those through a visit summary dataset.
ProcessCcdWithFakes and ImageDifferenceTask
These tasks are already deprecated in their entirety, or will soon be. It's not worth touching them.
See later comment (Jira refuses to link to it here).
ForcedPhotCcdTask
Again deprecate connections and config fields that control them.
Add a new visit summary input connection.
Again I do not think a config toggle for using calexp components is worth it, because we could pass those through visit summaries instead. Here I am less certain; given that it uses a skymap-based forcing catalog, I think this is a DRP-only task, and AP forced photometry would require a task with a different set of connections (at least) - but I'd like to have that assertion checked by the AP team. I don't think we want AP to have to run visit summaries, at least not with low latency, as they're a full-focal-plane sequence point.
GetTemplateTask
This doesn't have connections that take DRP-style final calibrations at all, just a TODO comment for DM-31292, but when that ticket is implemented, it should do so by adding a visit summary input connection as well as a config toggle to select between that and the existing "wcs" connection.
SubtractImagesTask
Deprecate the finalizedPsfApCorrCatalog connection and add a new visit summary one. This is just a rename, though it leaves the door open to using updated PhotoCalib and WCS in the future.
Similarly deprecate the doApplyFinalizedPsf and add a new doApplyFinalCalibrations (or similar) replacement, leaving room for using more than the PSF instead of implying that more config toggles should be added (I maintain that these tasks are either run in AP mode without any visit summaries or in DRP mode with all components coming from visit summaries; I can't think of anything in between).
faro (DetectorMeasurement and MatchedCatalogBase).
As in most cases above, deprecate connections and related configs. Add a visit summary input.
(Links are to DetectorMeasurement, but MatchedCatalogBase has identical blocks).
Here I am again less certain about whether we should have a toggle that lets these use the calexp inputs for AP usage. The Jeffrey Carlin?). I do not see any pipelines in drp_pipe, ap_pipe, or ap_verify that would need to use them, since they are only being included in pipelines with visit summaries already present.DM-35207 changes actually already assume we don't need that, since I think these tasks are ultimately going to be replaced before they're used in the AP pipeline and I don't see any of them in the AP pipelines now. But I'd very much like to have that confirmed (
What have I missed?
Are there any other tasks that should be similarly updated? DM-35207 will already replace some usage of the initial visitSummary with the finalVisitSummary in analysis_tools, analysis_drp, and HealSparsePropertyMapTask, but I believe those are really about the summary stats, not the exposure components. And I think all new analysis code that wants calibrated sources should do so via a calibrated-source dataset, not by reapplying the calibration objects from a visit summary to a source catalog itself. Are there any exceptions to that?
A comment below (Jira refuses to link to it) includes some later additions to this RFC's proposals; some changes have already been annotated above.
Attachments
Issue Links
- is triggering
-
DM-39854 Implement removals for RFC-901
- To Do
-
DM-39227 Implement deprecations for RFC-901
- Done
- relates to
-
DM-35207 Use final PSF models to determine inputs to coaddition
- Done
- mentioned in
-
Page Loading...
-
Page Loading...
-
Page Loading...
-
Page Loading...
-
Page Loading...
-
Page Loading...
-
Page Loading...
-
Page Loading...
-
Page Loading...
-
Page Loading...
-
Page Loading...
-
Page Loading...
-
Page Loading...
-
Page Loading...
They're almost substitutable, but finalVisitSummary has PSFs while visitSummary does not (at present). This didn't affect the current ticket because the only task that currently consumes those PSFs is MakeWarp, and in all current pipelines that is either configured to get them from finalVisitSummary or not get them from a visit summary of any kind. Many more tasks consume the final WCS and/or PhotoCalib, and for those (and many other things) visitSummary and finalVisitSummary are already substitutable.