Uploaded image for project: 'Data Management'
  1. Data Management
  2. DM-11136

Fine-tune calib construction mechanics

    XMLWordPrintable

    Details

    • Team:
      External

      Description

      The Calibs team has decided that calib construction should work in this way:

      • construct*.py writes the outputs to the rerun (butler output) directory.
      • ingestCalibs.py moves the calibs into the calib directory (with a link option for testing)

        Attachments

          Issue Links

            Activity

            Hide
            price Paul Price added a comment -

            ci_ctio0m9, which tests the calib construction mechanics, passes on Jenkins: https://ci.lsst.codes/job/stack-os-matrix/24978/label=centos-7,python=py2/console
            (It doesn't pass under python 3, because it uses scons, which doesn't yet work on python 3.)

            Show
            price Paul Price added a comment - ci_ctio0m9, which tests the calib construction mechanics, passes on Jenkins: https://ci.lsst.codes/job/stack-os-matrix/24978/label=centos-7,python=py2/console (It doesn't pass under python 3, because it uses scons, which doesn't yet work on python 3.)
            Hide
            price Paul Price added a comment -

            Nate Pease, would you please review this? Besides the changes listed below (in obs_base, pipe_tasks and pipe_drivers), there's also the master branch of the new package ci_ctio0m9. We can hand ci_ctio0m9 off to someone else if you feel it's a bit much, but I hope you, as lead butler developer, can look through the below changes.

            price@pap-laptop:~/LSST/obs_base (tickets/DM-11136=) $ git sub
            commit 5a05312d00e1f0e5bbb3b426bfc96a65bfd28e2e
            Author: Paul Price <price@astro.princeton.edu>
            Date:   Fri Jun 30 17:56:53 2017 -0400
             
                Write calib outputs to data root, not calib root
                
                This allows the user to review the results before they 'go live'
                and potentially get used by active processes.
             
             python/lsst/obs/base/cameraMapper.py |  2 +-
             python/lsst/obs/base/mapping.py      | 14 ++++++++++++--
             2 files changed, 13 insertions(+), 3 deletions(-)
             
             
            price@pap-laptop:~/LSST/pipe_tasks (tickets/DM-11136=) $ git sub-patch
            commit 25302c85c4b602295c416554a66a47733175a0d6
            Author: Paul Price <price@astro.princeton.edu>
            Date:   Mon Jul 3 14:28:52 2017 -0500
             
                ingestCalibs: change default mode to 'move'
                
                The standard pattern will be that calibs are created outside the main
                calib directory (e.g., in the rerun) so that they are not immediately
                available for use by active processes. Then they need to be transferred
                into the calib directory; 'move' is better than 'link' because we will
                want to blow away the directory where they were originally created.
             
            diff --git a/python/lsst/pipe/tasks/ingestCalibs.py b/python/lsst/pipe/tasks/ingestCalibs.py
            index ae9eb0d..02eb798 100644
            --- a/python/lsst/pipe/tasks/ingestCalibs.py
            +++ b/python/lsst/pipe/tasks/ingestCalibs.py
            @@ -182,7 +182,7 @@ class IngestCalibsArgumentParser(InputOnlyArgumentParser):
                     InputOnlyArgumentParser.__init__(self, *args, **kwargs)
                     self.add_argument("-n", "--dry-run", dest="dryrun", action="store_true",
                                       default=False, help="Don't perform any action?")
            -        self.add_argument("--mode", choices=["move", "copy", "link", "skip"], default="skip",
            +        self.add_argument("--mode", choices=["move", "copy", "link", "skip"], default="move",
                                       help="Mode of delivering the files to their destination")
                     self.add_argument("--create", action="store_true", help="Create new registry?")
                     self.add_argument("--validity", type=int, required=True, help="Calibration validity period (days)")
             
             
            price@pap-laptop:~/LSST/pipe_drivers (tickets/DM-11136=) $ git sub-patch
            commit 156514ea75549c672a5edb176654bb4d7aeb15f0
            Author: Paul Price <price@astro.princeton.edu>
            Date:   Fri Jun 30 17:29:56 2017 -0500
             
                constructCalibs: fix addMissingKeys usage
                
                addMissingKeys needs something to start with, so set it to what we already
                have, and then add what's missing.
             
            diff --git a/python/lsst/pipe/drivers/constructCalibs.py b/python/lsst/pipe/drivers/constructCalibs.py
            index 2585072..038e2a3 100644
            --- a/python/lsst/pipe/drivers/constructCalibs.py
            +++ b/python/lsst/pipe/drivers/constructCalibs.py
            @@ -386,6 +386,7 @@ class CalibTask(BatchPoolTask):
                     outputIdItemList = list(outputId.items())
                     for ccdName in ccdIdLists:
                         dataId = dict([(k, ccdName[i]) for i, k in enumerate(self.config.ccdKeys)])
            +            dataId.update(outputIdItemList)
                         self.addMissingKeys(dataId, butler)
                         dataId.update(outputIdItemList)
                         
            @@ -635,6 +636,7 @@ class CalibTask(BatchPoolTask):
                     """
                     # Check if we need to look up any keys that aren't in the output dataId
                     fullOutputId = {k: struct.ccdName[i] for i, k in enumerate(self.config.ccdKeys)}
            +        fullOutputId.update(outputId)
                     self.addMissingKeys(fullOutputId, cache.butler)
                     fullOutputId.update(outputId)  # must be after the call to queryMetadata
                     outputId = fullOutputId
            

            Show
            price Paul Price added a comment - Nate Pease , would you please review this? Besides the changes listed below (in obs_base, pipe_tasks and pipe_drivers), there's also the master branch of the new package ci_ctio0m9. We can hand ci_ctio0m9 off to someone else if you feel it's a bit much, but I hope you, as lead butler developer, can look through the below changes. price@pap-laptop:~/LSST/obs_base (tickets/DM-11136=) $ git sub commit 5a05312d00e1f0e5bbb3b426bfc96a65bfd28e2e Author: Paul Price <price@astro.princeton.edu> Date: Fri Jun 30 17:56:53 2017 -0400   Write calib outputs to data root, not calib root This allows the user to review the results before they 'go live' and potentially get used by active processes.   python/lsst/obs/base/cameraMapper.py | 2 +- python/lsst/obs/base/mapping.py | 14 ++++++++++++-- 2 files changed, 13 insertions(+), 3 deletions(-)     price@pap-laptop:~/LSST/pipe_tasks (tickets/DM-11136=) $ git sub-patch commit 25302c85c4b602295c416554a66a47733175a0d6 Author: Paul Price <price@astro.princeton.edu> Date: Mon Jul 3 14:28:52 2017 -0500   ingestCalibs: change default mode to 'move' The standard pattern will be that calibs are created outside the main calib directory (e.g., in the rerun) so that they are not immediately available for use by active processes. Then they need to be transferred into the calib directory; 'move' is better than 'link' because we will want to blow away the directory where they were originally created.   diff --git a/python/lsst/pipe/tasks/ingestCalibs.py b/python/lsst/pipe/tasks/ingestCalibs.py index ae9eb0d..02eb798 100644 --- a/python/lsst/pipe/tasks/ingestCalibs.py +++ b/python/lsst/pipe/tasks/ingestCalibs.py @@ -182,7 +182,7 @@ class IngestCalibsArgumentParser(InputOnlyArgumentParser): InputOnlyArgumentParser.__init__(self, *args, **kwargs) self.add_argument("-n", "--dry-run", dest="dryrun", action="store_true", default=False, help="Don't perform any action?") - self.add_argument("--mode", choices=["move", "copy", "link", "skip"], default="skip", + self.add_argument("--mode", choices=["move", "copy", "link", "skip"], default="move", help="Mode of delivering the files to their destination") self.add_argument("--create", action="store_true", help="Create new registry?") self.add_argument("--validity", type=int, required=True, help="Calibration validity period (days)")     price@pap-laptop:~/LSST/pipe_drivers (tickets/DM-11136=) $ git sub-patch commit 156514ea75549c672a5edb176654bb4d7aeb15f0 Author: Paul Price <price@astro.princeton.edu> Date: Fri Jun 30 17:29:56 2017 -0500   constructCalibs: fix addMissingKeys usage addMissingKeys needs something to start with, so set it to what we already have, and then add what's missing.   diff --git a/python/lsst/pipe/drivers/constructCalibs.py b/python/lsst/pipe/drivers/constructCalibs.py index 2585072..038e2a3 100644 --- a/python/lsst/pipe/drivers/constructCalibs.py +++ b/python/lsst/pipe/drivers/constructCalibs.py @@ -386,6 +386,7 @@ class CalibTask(BatchPoolTask): outputIdItemList = list(outputId.items()) for ccdName in ccdIdLists: dataId = dict([(k, ccdName[i]) for i, k in enumerate(self.config.ccdKeys)]) + dataId.update(outputIdItemList) self.addMissingKeys(dataId, butler) dataId.update(outputIdItemList) @@ -635,6 +636,7 @@ class CalibTask(BatchPoolTask): """ # Check if we need to look up any keys that aren't in the output dataId fullOutputId = {k: struct.ccdName[i] for i, k in enumerate(self.config.ccdKeys)} + fullOutputId.update(outputId) self.addMissingKeys(fullOutputId, cache.butler) fullOutputId.update(outputId) # must be after the call to queryMetadata outputId = fullOutputId
            Hide
            npease Nate Pease added a comment -

            Paul Price the code changes look fine, such as I understand them; the code changes and code in obs_base looks fine, the code in pipe_drivers and pipe_tasks looks fine, I assume you know it's the right thing to do.

            Can you please add a section to LDM-463 under Camera Mapper that summarizes the convention for calibs that is described here and in RFC-359? I think we need to have this behavior expectation/requirement documented. Thanks.

            Show
            npease Nate Pease added a comment - Paul Price the code changes look fine, such as I understand them; the code changes and code in obs_base looks fine, the code in pipe_drivers and pipe_tasks looks fine, I assume you know it's the right thing to do. Can you please add a section to LDM-463 under Camera Mapper that summarizes the convention for calibs that is described here and in RFC-359 ? I think we need to have this behavior expectation/requirement documented. Thanks.
            Hide
            npease Nate Pease added a comment -

            thanks for adding the blurb in LDM-463. looks good, I'm done reviewing.

            Show
            npease Nate Pease added a comment - thanks for adding the blurb in LDM-463. looks good, I'm done reviewing.
            Hide
            price Paul Price added a comment -

            Thanks, Nate.

            Merged to master.

            Show
            price Paul Price added a comment - Thanks, Nate. Merged to master.

              People

              Assignee:
              price Paul Price
              Reporter:
              price Paul Price
              Reviewers:
              Nate Pease
              Watchers:
              Colin Slater, Merlin Fisher-Levine, Nate Pease, Paul Price, Robert Lupton
              Votes:
              0 Vote for this issue
              Watchers:
              5 Start watching this issue

                Dates

                Created:
                Updated:
                Resolved:

                  CI Builds

                  No builds found.