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

Port HSC MPI driver for single-visit processing

    Details

    • Type: Story
    • Status: Done
    • Resolution: Done
    • Fix Version/s: None
    • Component/s: pipe_tasks
    • Labels:
      None
    • Story Points:
      6
    • Sprint:
      Science Pipelines DM-W16-6, DRP X16-1, DRP X16-2
    • Team:
      Data Release Production

      Description

      Transfer the reduceFrames.py script and the ProcessExposureTask it utilizes from hscPipe to a new package in the LSST stack (RFC-68 proposes calling this new package pool_tasks, but this isn't set in stone).

      We should probably rename either the driver script or the task (or both), so they agree; the lack of consistency is a historical artifact on the HSC side, and I think it's time to change that.

        Attachments

          Issue Links

            Activity

            Hide
            jbosch Jim Bosch added a comment -

            We should consider delaying this until DM-4255 is complete, as that's a complete rewrite of ProcessCcdTask, and the right solution here may just be to turn ProcessCcdTask into a BatchPoolTask rather than putting another layer around it.

            Show
            jbosch Jim Bosch added a comment - We should consider delaying this until DM-4255 is complete, as that's a complete rewrite of ProcessCcdTask , and the right solution here may just be to turn ProcessCcdTask into a BatchPoolTask rather than putting another layer around it.
            Hide
            swinbank John Swinbank added a comment -

            We discussed this at sprint planning of 2016-03-31. We agreed that the stopping condition for this ticket is to provide a layer capable of coordinating multiple executions of processCcd (covering the entire exposure), not to perform "global" processing as is currently carried out on HSC.

            Show
            swinbank John Swinbank added a comment - We discussed this at sprint planning of 2016-03-31. We agreed that the stopping condition for this ticket is to provide a layer capable of coordinating multiple executions of processCcd (covering the entire exposure), not to perform "global" processing as is currently carried out on HSC.
            Hide
            rowen Russell Owen added a comment -

            This basically looks fine, though I requested some cleanups on github

            Show
            rowen Russell Owen added a comment - This basically looks fine, though I requested some cleanups on github
            Hide
            nlust Nate Lust added a comment -

            Russell Owen I think I have address everything significant. I am going to ask Dominique Boutigny if he would look at the new obs_cfht branch and Hsin-Fang Chiang if she would look at the obs_decam branch. Kian-Tat Lim In this ticket I only plan on adding support to run the single frame driver task to HSC, DECAM, and CFHT. The LSST telescopes have a different dataId structure that will need to be adapted in the future, and SDSS does not really fit into the visit, ccd model of things. This does not preclude support being added in the future, but it a bit outside of this porting ticket. Do you have any objections?

            Show
            nlust Nate Lust added a comment - Russell Owen I think I have address everything significant. I am going to ask Dominique Boutigny if he would look at the new obs_cfht branch and Hsin-Fang Chiang if she would look at the obs_decam branch. Kian-Tat Lim In this ticket I only plan on adding support to run the single frame driver task to HSC, DECAM, and CFHT. The LSST telescopes have a different dataId structure that will need to be adapted in the future, and SDSS does not really fit into the visit, ccd model of things. This does not preclude support being added in the future, but it a bit outside of this porting ticket. Do you have any objections?
            Hide
            hchiang2 Hsin-Fang Chiang added a comment -

            I confirm that singleFrameDriver runs with raw DECam data, with the two minor corrections noted in the obs_decam PR.

            Show
            hchiang2 Hsin-Fang Chiang added a comment - I confirm that singleFrameDriver runs with raw DECam data, with the two minor corrections noted in the obs_decam PR.
            Hide
            rowen Russell Owen added a comment - - edited

            I do have another request (or set of related requests), though you can file a ticket and do it later if you need to:

            • add a doc directory with the usual content, so the doc strings get generated by Doxygen
            • Add a main.dox overview file to the doc directory so users have some idea what this package is for and how to use it
            • Make sure all doc strings start with """! do Doxygen parses the doxygen commands

            Aside from that it looks fine to me.

            Show
            rowen Russell Owen added a comment - - edited I do have another request (or set of related requests), though you can file a ticket and do it later if you need to: add a doc directory with the usual content, so the doc strings get generated by Doxygen Add a main.dox overview file to the doc directory so users have some idea what this package is for and how to use it Make sure all doc strings start with """! do Doxygen parses the doxygen commands Aside from that it looks fine to me.
            Hide
            boutigny Dominique Boutigny added a comment -

            I tried it with obs_cfht. It runs but never ends ! It is looping over the CCDs indefinitely

            Show
            boutigny Dominique Boutigny added a comment - I tried it with obs_cfht. It runs but never ends ! It is looping over the CCDs indefinitely
            Hide
            hchiang2 Hsin-Fang Chiang added a comment -

            This is a long shot, Dominique Boutigny would you mind trying branch u/hfc/DM-3368? I am guessing obs_cfht may need a fix that was previously done to obs_decam.

            Show
            hchiang2 Hsin-Fang Chiang added a comment - This is a long shot, Dominique Boutigny would you mind trying branch u/hfc/ DM-3368 ? I am guessing obs_cfht may need a fix that was previously done to obs_decam.
            Hide
            nlust Nate Lust added a comment -

            Hsin-Fang ChiangDominique Boutigny I fixed those issues before Dominique started looking at this. I am seeing the behavior on lsst-dev as well and am looking into it. I think there might be additional cfht specific code that might need to go in.

            Show
            nlust Nate Lust added a comment - Hsin-Fang Chiang Dominique Boutigny I fixed those issues before Dominique started looking at this. I am seeing the behavior on lsst-dev as well and am looking into it. I think there might be additional cfht specific code that might need to go in.
            Hide
            hchiang2 Hsin-Fang Chiang added a comment -

            I suspected a possible bug equivalent to DM-5586 for obs_cfht (a MegacamMapper bug, not a pipe_drivers bug). If I understand correctly, with that bug, it would loop (the number of extension)*(the number of state) times more than it should. It seems to me that Megacam data can have 36 extensions and 2 states, so at most 36*2 = 72 times.

            If it goes on indefinitely (more than 72 times), please disregard my random guess....

            Show
            hchiang2 Hsin-Fang Chiang added a comment - I suspected a possible bug equivalent to DM-5586 for obs_cfht (a MegacamMapper bug, not a pipe_drivers bug). If I understand correctly, with that bug, it would loop (the number of extension)*(the number of state) times more than it should. It seems to me that Megacam data can have 36 extensions and 2 states, so at most 36*2 = 72 times. If it goes on indefinitely (more than 72 times), please disregard my random guess....
            Hide
            price Paul Price added a comment -

            It is absolutely the same kind of problem as DM-5586.

            Before:

            >>> butler = Butler('/raid/nlust/repos/validation_data_cfht/data/')
            >>> refList = list(butler.subset(datasetType="raw", level="visit", dataId={'visit': 850587}))
            >>> len(refList)
            36
            

            Fix:

            price@lsst-dev:~/LSST/obs/cfht (master *) $ git diff
            diff --git a/policy/MegacamMapper.paf b/policy/MegacamMapper.paf
            index 25a1e3a..ded990a 100644
            --- a/policy/MegacamMapper.paf
            +++ b/policy/MegacamMapper.paf
            @@ -14,7 +14,7 @@ levels: {
                 # Keys that are NOT relevant for a particular level
                 skyTile: "visit" "ccd"
                 tract: "patch"
            -    visit: "ccd"
            +    visit: "ccd" "extension" "state"
                 sensor: "none"
             }
             defaultLevel: "Ccd"
            

            After:

            >>> butler = Butler('/raid/nlust/repos/validation_data_cfht/data/')
            >>> refList = list(butler.subset(datasetType="raw", level="visit", dataId={'visit': 850587}))
            >>> len(refList)
            1
            

            Show
            price Paul Price added a comment - It is absolutely the same kind of problem as DM-5586 . Before: >>> butler = Butler('/raid/nlust/repos/validation_data_cfht/data/') >>> refList = list(butler.subset(datasetType="raw", level="visit", dataId={'visit': 850587})) >>> len(refList) 36 Fix: price@lsst-dev:~/LSST/obs/cfht (master *) $ git diff diff --git a/policy/MegacamMapper.paf b/policy/MegacamMapper.paf index 25a1e3a..ded990a 100644 --- a/policy/MegacamMapper.paf +++ b/policy/MegacamMapper.paf @@ -14,7 +14,7 @@ levels: {      # Keys that are NOT relevant for a particular level      skyTile: "visit" "ccd"      tract: "patch" -    visit: "ccd" +    visit: "ccd" "extension" "state"      sensor: "none" } defaultLevel: "Ccd" After: >>> butler = Butler('/raid/nlust/repos/validation_data_cfht/data/') >>> refList = list(butler.subset(datasetType="raw", level="visit", dataId={'visit': 850587})) >>> len(refList) 1
            Hide
            price Paul Price added a comment -

            Oh, my fix is exactly the same as Hsin-Fang Chiang's.

            These problems are coming up because no-one has cared about getting visit-level data refs for these cameras before now.

            Show
            price Paul Price added a comment - Oh, my fix is exactly the same as Hsin-Fang Chiang 's. These problems are coming up because no-one has cared about getting visit-level data refs for these cameras before now.
            Hide
            nlust Nate Lust added a comment -

            Thanks all for catching that all, and I appologize about not getting what you meant at first hsin-fang, I'm very foggy today. I am lucky to have other team members who are great. I will put these fixes on my branch first thing Monday.

            Show
            nlust Nate Lust added a comment - Thanks all for catching that all, and I appologize about not getting what you meant at first hsin-fang, I'm very foggy today. I am lucky to have other team members who are great. I will put these fixes on my branch first thing Monday.
            Hide
            nlust Nate Lust added a comment -

            I added the fix to my obs_cfht branch, and everything now runs to completion. Dominique Boutigny can you verify? And if so do you have any other review comments?

            Show
            nlust Nate Lust added a comment - I added the fix to my obs_cfht branch, and everything now runs to completion. Dominique Boutigny can you verify? And if so do you have any other review comments?
            Hide
            boutigny Dominique Boutigny added a comment -

            I confirm that this is now running and ending... with the latest mods in obs_cfht.
            Just one question :

            • How can I pass a configuration parameter file as I do with processCcd with the --configfile parameter ?
            Show
            boutigny Dominique Boutigny added a comment - I confirm that this is now running and ending... with the latest mods in obs_cfht. Just one question : How can I pass a configuration parameter file as I do with processCcd with the --configfile parameter ?
            Hide
            nlust Nate Lust added a comment -

            You should be able to do it just the same way, just noting that processccd is a subtask (hence sub configuration) of SingleFrameDriver test. So your configuration file would be something like config.processCcd.<configuration variable> = <value> instead of just config.<configuration variable> = <value>. Everything will just be relative to the configuration of the task you are running.

            Show
            nlust Nate Lust added a comment - You should be able to do it just the same way, just noting that processccd is a subtask (hence sub configuration) of SingleFrameDriver test. So your configuration file would be something like config.processCcd.<configuration variable> = <value> instead of just config.<configuration variable> = <value>. Everything will just be relative to the configuration of the task you are running.
            Hide
            price Paul Price added a comment -

            Dominique Boutigny, put in your obs package's config/singleFrameDriver.py:

            import os.path
            from lsst.utils import getPackageDir
            config.processCcd.load(os.path.join(getPackageDir("obs_subaru"), "config", "processCcd.py"))
            

            This ensures you're using the same overrides in the singleFrameDriver as you use for processCcd.

            If you've got an override file that's not being automatically loaded from your obs package, you can put this at the top:

            try:
                config = config.processCcd
            except:
                pass
            

            With this, you can use the overrides with either the singleFrameDriver or processCcd.

            Show
            price Paul Price added a comment - Dominique Boutigny , put in your obs package's config/singleFrameDriver.py: import os.path from lsst.utils import getPackageDir config.processCcd.load(os.path.join(getPackageDir("obs_subaru"), "config", "processCcd.py")) This ensures you're using the same overrides in the singleFrameDriver as you use for processCcd. If you've got an override file that's not being automatically loaded from your obs package, you can put this at the top: try: config = config.processCcd except: pass With this, you can use the overrides with either the singleFrameDriver or processCcd.
            Hide
            nlust Nate Lust added a comment -

            Paul Price, The config file that I built for cfht already has that in it, so the only overrides Dominique Boutigny should need are run specific things if he wants to change an option between runs or for a specific purpose.

            Show
            nlust Nate Lust added a comment - Paul Price , The config file that I built for cfht already has that in it, so the only overrides Dominique Boutigny should need are run specific things if he wants to change an option between runs or for a specific purpose.
            Hide
            nlust Nate Lust added a comment -

            merged to master

            Show
            nlust Nate Lust added a comment - merged to master

              People

              • Assignee:
                nlust Nate Lust
                Reporter:
                jbosch Jim Bosch
                Reviewers:
                Russell Owen
                Watchers:
                Dominique Boutigny, Hsin-Fang Chiang, Jim Bosch, John Swinbank, Nate Lust, Paul Price, Russell Owen
              • Votes:
                0 Vote for this issue
                Watchers:
                7 Start watching this issue

                Dates

                • Created:
                  Updated:
                  Resolved:

                  Summary Panel