Merge pull request #431 from lsst/tickets/DM-27147

DM-27147: consolidate skymap definitions in Gen3

bin.src/makeGen3Skymap.py

@@ -30,8 +30,6 @@
 # Build a parser for command line arguments
 parser = argparse.ArgumentParser(description="Make a SkyMap and add it to a gen3 repository.")
 parser.add_argument("butler", metavar="Butler", type=str, help="Path to a gen3 butler")
-parser.add_argument("collection", type=str, metavar="Collection",
-                    help="Name of the Butler collection the SkyMap should be inserted into")
 parser.add_argument("-C", "--config-file", dest="configFile",
                     help="Path to a config file overrides file")
 
@@ -46,5 +44,5 @@
 
 # Construct the SkyMap Creation task and run it
 skyMapTask = MakeGen3SkyMapTask(config=config)
-butler = Butler(args.butler, run=args.collection)
+butler = Butler(args.butler, writeable=True)
 skyMapTask.run(butler)

python/lsst/pipe/tasks/assembleCoadd.py

@@ -37,6 +37,7 @@
 import lsst.log as log
 import lsstDebug
 import lsst.utils as utils
+from lsst.skymap import BaseSkyMap
 from .coaddBase import CoaddBaseTask, SelectDataIdContainer, makeSkyInfo, makeCoaddSuffix
 from .interpImage import InterpImageTask
 from .scaleZeroPoint import ScaleZeroPointTask
@@ -69,7 +70,7 @@ class AssembleCoaddConnections(pipeBase.PipelineTaskConnections,
     )
     skyMap = pipeBase.connectionTypes.Input(
         doc="Input definition of geometry/bbox and projection/wcs for coadded exposures",
-        name="{inputCoaddName}Coadd_skyMap",
+        name=BaseSkyMap.SKYMAP_DATASET_TYPE_NAME,
         storageClass="SkyMap",
         dimensions=("skymap", ),
     )

python/lsst/pipe/tasks/cli/cmd/commands.py

@@ -45,12 +45,12 @@
               callback=split_commas,
               metavar=typeStrAcceptsMultiple,
               required=True)
-@click.option("--out-collection",
-              help=("The collection to write the skymap to."),
-              type=str, default="skymaps", show_default=True)
 @click.option("--skymap-id",
               help=("The identifier of the skymap to write."),
               type=str, default="discrete", show_default=True)
+@click.option("--old-skymap-id",
+              help=("The identifier of the previous skymap to append to, if config.doAppend is True."),
+              type=str, default=None)
 def make_discrete_skymap(*args, **kwargs):
     """Define a discrete skymap from calibrated exposures in the butler registry."""
     cli_handle_exception(script.makeDiscreteSkyMap, *args, **kwargs)

python/lsst/pipe/tasks/dcrAssembleCoadd.py

@@ -34,6 +34,7 @@
 import lsst.pex.config as pexConfig
 import lsst.pipe.base as pipeBase
 import lsst.utils as utils
+from lsst.skymap import BaseSkyMap
 from .assembleCoadd import (AssembleCoaddTask,
                             CompareWarpAssembleCoaddConfig,
                             CompareWarpAssembleCoaddTask)
@@ -61,7 +62,7 @@ class DcrAssembleCoaddConnections(pipeBase.PipelineTaskConnections,
     )
     skyMap = pipeBase.connectionTypes.Input(
         doc="Input definition of geometry/bbox and projection/wcs for coadded exposures",
-        name="{inputCoaddName}Coadd_skyMap",
+        name=BaseSkyMap.SKYMAP_DATASET_TYPE_NAME,
         storageClass="SkyMap",
         dimensions=("skymap", ),
     )

python/lsst/pipe/tasks/imageDifference.py

@@ -43,6 +43,7 @@
 import lsst.ip.diffim.diffimTools as diffimTools
 import lsst.ip.diffim.utils as diUtils
 import lsst.afw.display as afwDisplay
+from lsst.skymap import BaseSkyMap
 
 __all__ = ["ImageDifferenceConfig", "ImageDifferenceTask"]
 FwhmPerSigma = 2*math.sqrt(2*math.log(2))
@@ -73,7 +74,7 @@ class ImageDifferenceTaskConnections(pipeBase.PipelineTaskConnections,
 
     skyMap = pipeBase.connectionTypes.Input(
         doc="Input definition of geometry/bbox and projection/wcs for template exposures",
-        name="{skyMapName}Coadd_skyMap",
+        name=BaseSkyMap.SKYMAP_DATASET_TYPE_NAME,
         dimensions=("skymap", ),
         storageClass="SkyMap",
     )

python/lsst/pipe/tasks/makeCoaddTempExp.py

@@ -30,6 +30,7 @@
 import lsst.log as log
 import lsst.utils as utils
 from lsst.meas.algorithms import CoaddPsf, CoaddPsfConfig
+from lsst.skymap import BaseSkyMap
 from .coaddBase import CoaddBaseTask, makeSkyInfo
 from .warpAndPsfMatch import WarpAndPsfMatchTask
 from .coaddHelpers import groupPatchExposures, getGroupDataRef
@@ -596,7 +597,7 @@ class MakeWarpConnections(pipeBase.PipelineTaskConnections,
     )
     skyMap = cT.Input(
         doc="Input definition of geometry/bbox and projection/wcs for warped exposures",
-        name="{coaddName}Coadd_skyMap",
+        name=BaseSkyMap.SKYMAP_DATASET_TYPE_NAME,
         storageClass="SkyMap",
         dimensions=("skymap",),
     )

python/lsst/pipe/tasks/makeGen3SkyMap.py

@@ -20,20 +20,12 @@
 # along with this program.  If not, see <https://www.gnu.org/licenses/>.
 import lsst.pex.config as pexConfig
 import lsst.pipe.base as pipeBase
-from lsst.daf.butler import DatasetType
 from lsst.skymap import skyMapRegistry
 
-from sqlalchemy.exc import IntegrityError
-
 
 class MakeGen3SkyMapConfig(pexConfig.Config):
     """Config for MakeGen3SkyMapTask
     """
-    datasetTypeName = pexConfig.Field(
-        doc="Name assigned to created skymap in butler registry",
-        dtype=str,
-        default="deepCoadd_skyMap",
-    )
     name = pexConfig.Field(
         doc="Name assigned to created skymap in butler registry",
         dtype=str,
@@ -77,19 +69,7 @@ def run(self, butler):
         """
         skyMap = self.config.skyMap.apply()
         skyMap.logSkyMapInfo(self.log)
-        skyMapHash = skyMap.getSha1()
-        self.log.info(f"Registering dataset type {self.config.datasetTypeName}.")
-        butler.registry.registerDatasetType(DatasetType(name=self.config.datasetTypeName,
-                                                        dimensions=["skymap"],
-                                                        storageClass="SkyMap",
-                                                        universe=butler.registry.dimensions))
-        self.log.info(f"Inserting SkyMap {self.config.name} with hash={skyMapHash}")
-        with butler.registry.transaction():
-            try:
-                skyMap.register(self.config.name, butler.registry)
-            except IntegrityError as err:
-                raise RuntimeError("A skymap with the same name or hash already exists.") from err
-            butler.put(skyMap, self.config.datasetTypeName, {"skymap": self.config.name})
+        skyMap.register(self.config.name, butler)
         return pipeBase.Struct(
             skyMap=skyMap
         )

python/lsst/pipe/tasks/mergeDetections.py

@@ -30,6 +30,7 @@
 import lsst.afw.table as afwTable
 
 from lsst.meas.algorithms import SkyObjectsTask
+from lsst.skymap import BaseSkyMap
 from lsst.pex.config import Config, Field, ListField, ConfigurableField, ConfigField
 from lsst.pipe.base import (CmdLineTask, PipelineTask, PipelineTaskConfig, Struct,
                             PipelineTaskConnections)
@@ -68,7 +69,7 @@ class MergeDetectionsConnections(PipelineTaskConnections,
 
     skyMap = cT.Input(
         doc="SkyMap to be used in merging",
-        name="{inputCoaddName}Coadd_skyMap",
+        name=BaseSkyMap.SKYMAP_DATASET_TYPE_NAME,
         storageClass="SkyMap",
         dimensions=("skymap",),
     )

python/lsst/pipe/tasks/multiBand.py

@@ -39,6 +39,7 @@
 import lsst.afw.table as afwTable
 import lsst.afw.math as afwMath
 from lsst.daf.base import PropertyList
+from lsst.skymap import BaseSkyMap
 
 from .mergeDetections import MergeDetectionsConfig, MergeDetectionsTask  # noqa: F401
 from .mergeMeasurements import MergeMeasurementsConfig, MergeMeasurementsTask  # noqa: F401
@@ -614,7 +615,7 @@ class MeasureMergedCoaddSourcesConnections(PipelineTaskConnections,
     )
     skyMap = cT.Input(
         doc="SkyMap to use in processing",
-        name="{inputCoaddName}Coadd_skyMap",
+        name=BaseSkyMap.SKYMAP_DATASET_TYPE_NAME,
         storageClass="SkyMap",
         dimensions=("skymap",),
     )

python/lsst/pipe/tasks/script/makeDiscreteSkyMap.py

@@ -19,13 +19,14 @@
 # You should have received a copy of the GNU General Public License
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
-from lsst.daf.butler import Butler, DatasetType
+from lsst.daf.butler import Butler
+from lsst.skymap import BaseSkyMap
 from lsst.pipe.tasks.makeDiscreteSkyMap import MakeDiscreteSkyMapTask, MakeDiscreteSkyMapConfig
 from lsst.obs.base.utils import getInstrument
 
 
 def makeDiscreteSkyMap(repo, config_file, collections, instrument,
-                       out_collection='skymaps', skymap_id='discrete'):
+                       skymap_id='discrete', old_skymap_id=None):
     """Implements the command line interface `butler make-discrete-skymap` subcommand,
     should only be called by command line tools and unit test code that tests
     this function.
@@ -45,29 +46,31 @@ def makeDiscreteSkyMap(repo, config_file, collections, instrument,
         with the calibrated exposures.
     instrument : `str`
         The name or fully-qualified class name of an instrument.
-    out_collection : `str`, optional
-        The name of the collection to save the skymap to.  Default is 'skymaps'.
     skymap_id : `str`, optional
         The identifier of the skymap to save.  Default is 'discrete'.
+    old_skymap_id : `str`, optional
+        The identifer of the skymap to append to.  Must differ from
+        ``skymap_id``.  Ignored unless ``config.doAppend=True``.
     """
-    butler = Butler(repo, collections=collections, writeable=True, run=out_collection)
+    butler = Butler(repo, collections=collections, writeable=True)
     instr = getInstrument(instrument, butler.registry)
     config = MakeDiscreteSkyMapConfig()
     instr.applyConfigOverrides(MakeDiscreteSkyMapTask._DefaultName, config)
 
     if config_file is not None:
         config.load(config_file)
-    skymap_name = config.coaddName + "Coadd_skyMap"
+    # The coaddName for a SkyMap is only relevant in Gen2, and we completely
+    # ignore it here; once Gen2 is gone it can be removed.
     oldSkyMap = None
     if config.doAppend:
-        if out_collection in collections:
-            raise ValueError(f"Cannot overwrite dataset.  If appending, specify an output "
-                             f"collection not in the input collections.")
-        dataId = {'skymap': skymap_id}
+        if old_skymap_id is None:
+            raise ValueError("old_skymap_id must be provided if config.doAppend is True.")
+        dataId = {'skymap': old_skymap_id}
         try:
-            oldSkyMap = butler.get(skymap_name, collections=collections, dataId=dataId)
+            oldSkyMap = butler.get(BaseSkyMap.SKYMAP_DATASET_TYPE_NAME, collections=collections,
+                                   dataId=dataId)
         except LookupError as e:
-            msg = (f"Could not find seed skymap for {skymap_name} with dataId {dataId} "
+            msg = (f"Could not find seed skymap with dataId {dataId} "
                    f"in collections {collections} but doAppend is {config.doAppend}.  Aborting...")
             raise LookupError(msg, *e.args[1:])
 
@@ -76,11 +79,6 @@ def makeDiscreteSkyMap(repo, config_file, collections, instrument,
                          for ref in datasets]
     task = MakeDiscreteSkyMapTask(config=config)
     result = task.run(wcs_md_tuple_list, oldSkyMap)
-    skymap_dataset_type = DatasetType(skymap_name, dimensions=["skymap", ],
-                                      universe=butler.registry.dimensions,
-                                      storageClass="SkyMap")
-    butler.registry.registerDatasetType(skymap_dataset_type)
-    if config.doAppend:
-        # By definition if appending the dataset has already been registered
-        result.skyMap.register(skymap_id, butler.registry)
-    butler.put(result.skyMap, skymap_name, dataId={'skymap': skymap_id})
+    result.skyMap.register(skymap_id, butler)
+    butler.put(result.skyMap, BaseSkyMap.SKYMAP_DATASET_TYPE_NAME, dataId={'skymap': skymap_id},
+               run=BaseSkyMap.SKYMAP_RUN_COLLECTION_NAME)

tests/test_cliCmdMakeDiscreteSkymap.py

@@ -46,24 +46,24 @@ def test_repoBasic(self):
                        "here", "a.b.c"],
                       self.makeExpected(repo="here",
                                         collections=("foo/bar", "baz"),
-                                        out_collection="skymaps",
                                         skymap_id="discrete",
-                                        instrument="a.b.c"))
+                                        instrument="a.b.c",
+                                        old_skymap_id=None))
 
     def test_all(self):
         """Test all the arguments."""
         self.run_test(["make-discrete-skymap",
                        "--collections", "foo/bar,baz",
                        "--config-file", "/path/to/config",
                        "--collections", "boz",
-                       "--out-collection", "biz",
                        "--skymap-id", "wiz",
+                       "--old-skymap-id", "nuz",
                        "here", "a.b.c"],
                       self.makeExpected(repo="here",
                                         instrument="a.b.c",
                                         config_file="/path/to/config",
-                                        out_collection="biz",
                                         skymap_id="wiz",
+                                        old_skymap_id="nuz",
                                         # The list of collections must be in
                                         # exactly the same order as it is
                                         # passed in the list of arguments to