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

Allow controller = 'H' for exposure_id calculation

    XMLWordPrintable

    Details

    • Type: Improvement
    • Status: Done
    • Resolution: Done
    • Fix Version/s: None
    • Component/s: obs_lsst
    • Labels:

      Description

      To allow ingest of phosim-simulated data while using lsstCam mapper we suggest to add  controller = 'H'.  This is to guarantee unique obsid values for phosim data (Tim Jennes's comment).  If this is done, currently the translator returns (using  obs_lsst version w.2020.48-1-gf0a7a3e )

      translate_header.py -p lsst.obs.lsst.translators MC_H_20000217_000032_R00_SW0.fits 
       
      		 
      Analyzing MC_H_20000217_000032_R00_SW0.fits...
      		 
      ValueError("Supplied controller, 'H' is neither 'O' nor 'C'")

      An update is needed to allow for controller = 'H' . It affects the  `exposure_id` method in https://github.com/lsst/obs_lsst/blob/master/python/lsst/obs/lsst/translators/lsst.py  

      It doesn't clash with other definitions https://confluence.lsstcorp.org/display/SYSENG/Image+naming+and+visit+definition+conventions (K-T Lim's comment). 

       

        Attachments

          Issue Links

          is child task of

          Story - Created by Jira Software - do not edit or delete. Issue type for a user story. DM-28556 Adapt the Latest File Format of Corner WFS

          • Done

            Activity

            Ascending order - Click to sort in descending order
            No builds found.
            ksuberlak Krzysztof Suberlak created issue -
            ksuberlak Krzysztof Suberlak made changes -
            Field Original Value New Value
            Link This issue blocks DM-26836 [ DM-26836 ]
            ksuberlak Krzysztof Suberlak made changes -
            Description To allow ingest of phosim-simulated data while using lsstCam mapper, it was suggested to set controller = 'P'. If this is done, currently the translator returns 
            {code:java}
            translate_header.py -p lsst.obs.lsst.translators MC_P_20000217_000032_R00_SW0.fits

            Analyzing MC_P_20000217_000032_R00_SW0.fits...
            ValueError("Supplied controller, 'P' is neither 'O' nor 'C'")
            {code}
            It pertains to the  `exposure_id` method in [https://github.com/lsst/obs_lsst/blob/master/python/lsst/obs/lsst/translators/lsst.py]  

                          		To allow ingest of phosim-simulated data while using lsstCam mapper, it was suggested to set controller = 'P'. If this is done, currently the translator returns (using  obs_lsst version w.2020.48-1-gf0a7a3e )
            {code:java}translate_header.py -p lsst.obs.lsst.translators MC_P_20000217_000032_R00_SW0.fits

            Analyzing MC_P_20000217_000032_R00_SW0.fits...
            ValueError("Supplied controller, 'P' is neither 'O' nor 'C'")
            {code}
            It pertains to the  `exposure_id` method in [https://github.com/lsst/obs_lsst/blob/master/python/lsst/obs/lsst/translators/lsst.py]  

             
            ksuberlak Krzysztof Suberlak made changes -
            Description To allow ingest of phosim-simulated data while using lsstCam mapper, it was suggested to set controller = 'P'. If this is done, currently the translator returns (using  obs_lsst version w.2020.48-1-gf0a7a3e )
            {code:java}translate_header.py -p lsst.obs.lsst.translators MC_P_20000217_000032_R00_SW0.fits

            Analyzing MC_P_20000217_000032_R00_SW0.fits...
            ValueError("Supplied controller, 'P' is neither 'O' nor 'C'")
            {code}
            It pertains to the  `exposure_id` method in [https://github.com/lsst/obs_lsst/blob/master/python/lsst/obs/lsst/translators/lsst.py]  

                          		To allow ingest of phosim-simulated data while using lsstCam mapper, it was suggested to set controller = 'P'. If this is done, currently the translator returns (using  obs_lsst version w.2020.48-1-gf0a7a3e )
            {code:java}translate_header.py -p lsst.obs.lsst.translators MC_P_20000217_000032_R00_SW0.fits

            Analyzing MC_P_20000217_000032_R00_SW0.fits...
            ValueError("Supplied controller, 'P' is neither 'O' nor 'C'")
            {code}
            An update is needed to allow for controller = 'P' . It affects the  `exposure_id` method in [https://github.com/lsst/obs_lsst/blob/master/python/lsst/obs/lsst/translators/lsst.py]  

             
            ksuberlak Krzysztof Suberlak made changes -
            Status To Do [ 10001 ] In Progress [ 3 ]
            ksuberlak Krzysztof Suberlak made changes -
            Reviewers Kian-Tat Lim, Robert Lupton [ ktl, rhl ]
            Status In Progress [ 3 ] In Review [ 10004 ]
            ktl Kian-Tat Lim made changes -
            Assignee Krzysztof Suberlak [ ksuberlak ]
            Hide
            Permalink
            ktl Kian-Tat Lim added a comment -

            Generally we only assign one code reviewer. https://developer.lsst.io/work/flow.html#assign-a-reviewer

            The changes in the PR look good. An additional comment would be welcomed, as suggested in the PR. Ideally there would be a test as well; it looks like there may be some "controller = C" tests that could be emulated like https://github.com/lsst/obs_lsst/blob/master/tests/test_parsetask.py#L110-L132

            Also make sure you test this in CI: https://developer.lsst.io/work/flow.html#testing-with-jenkins

            Show
            ktl Kian-Tat Lim added a comment - Generally we only assign one code reviewer. https://developer.lsst.io/work/flow.html#assign-a-reviewer The changes in the PR look good. An additional comment would be welcomed, as suggested in the PR. Ideally there would be a test as well; it looks like there may be some "controller = C" tests that could be emulated like https://github.com/lsst/obs_lsst/blob/master/tests/test_parsetask.py#L110-L132 Also make sure you test this in CI: https://developer.lsst.io/work/flow.html#testing-with-jenkins
            Hide
            Permalink
            ktl Kian-Tat Lim added a comment -

            And the typical process for multiple reviewers is for each reviewer to remove their name from the reviewers list when done; the last sets the Jira issue to "Review Complete" state. Accordingly, I am removing myself.

            Show
            ktl Kian-Tat Lim added a comment - And the typical process for multiple reviewers is for each reviewer to remove their name from the reviewers list when done; the last sets the Jira issue to "Review Complete" state. Accordingly, I am removing myself.
            ktl Kian-Tat Lim made changes -
            Reviewers Kian-Tat Lim, Robert Lupton [ ktl, rhl ] Robert Lupton [ rhl ]
            ksuberlak Krzysztof Suberlak made changes -
            Summary Allow controller = 'P' for exposure_id calculation Allow controller = 'H' for exposure_id calculation
            ksuberlak Krzysztof Suberlak made changes -
            Description To allow ingest of phosim-simulated data while using lsstCam mapper, it was suggested to set controller = 'P'. If this is done, currently the translator returns (using  obs_lsst version w.2020.48-1-gf0a7a3e )
            {code:java}translate_header.py -p lsst.obs.lsst.translators MC_P_20000217_000032_R00_SW0.fits

            Analyzing MC_P_20000217_000032_R00_SW0.fits...
            ValueError("Supplied controller, 'P' is neither 'O' nor 'C'")
            {code}
            An update is needed to allow for controller = 'P' . It affects the  `exposure_id` method in [https://github.com/lsst/obs_lsst/blob/master/python/lsst/obs/lsst/translators/lsst.py]  

                          		To allow ingest of phosim-simulated data while using lsstCam mapper we suggest to add  controller = 'H'.  This is to guarantee unique obsid values for phosim data (Tim Jennes's comment).  If this is done, currently the translator returns (using  obs_lsst version w.2020.48-1-gf0a7a3e )
            {code:java}translate_header.py -p lsst.obs.lsst.translators MC_H_20000217_000032_R00_SW0.fits

            Analyzing MC_H_20000217_000032_R00_SW0.fits...
            ValueError("Supplied controller, 'P' is neither 'O' nor 'C'")
            {code}
            An update is needed to allow for controller = 'H' . It affects the  `exposure_id` method in [https://github.com/lsst/obs_lsst/blob/master/python/lsst/obs/lsst/translators/lsst.py]  

            It doesn't clash with other definitions [https://confluence.lsstcorp.org/display/SYSENG/Image+naming+and+visit+definition+conventions] (K-T Lim's comment). 

             
            ksuberlak Krzysztof Suberlak made changes -
            Description To allow ingest of phosim-simulated data while using lsstCam mapper we suggest to add  controller = 'H'.  This is to guarantee unique obsid values for phosim data (Tim Jennes's comment).  If this is done, currently the translator returns (using  obs_lsst version w.2020.48-1-gf0a7a3e )
            {code:java}translate_header.py -p lsst.obs.lsst.translators MC_H_20000217_000032_R00_SW0.fits

            Analyzing MC_H_20000217_000032_R00_SW0.fits...
            ValueError("Supplied controller, 'P' is neither 'O' nor 'C'")
            {code}
            An update is needed to allow for controller = 'H' . It affects the  `exposure_id` method in [https://github.com/lsst/obs_lsst/blob/master/python/lsst/obs/lsst/translators/lsst.py]  

            It doesn't clash with other definitions [https://confluence.lsstcorp.org/display/SYSENG/Image+naming+and+visit+definition+conventions] (K-T Lim's comment). 

                          		To allow ingest of phosim-simulated data while using lsstCam mapper we suggest to add  controller = 'H'.  This is to guarantee unique obsid values for phosim data (Tim Jennes's comment).  If this is done, currently the translator returns (using  obs_lsst version w.2020.48-1-gf0a7a3e )
            {code:java}translate_header.py -p lsst.obs.lsst.translators MC_H_20000217_000032_R00_SW0.fits

            Analyzing MC_H_20000217_000032_R00_SW0.fits...
            ValueError("Supplied controller, 'H' is neither 'O' nor 'C'")
            {code}
            An update is needed to allow for controller = 'H' . It affects the  `exposure_id` method in [https://github.com/lsst/obs_lsst/blob/master/python/lsst/obs/lsst/translators/lsst.py]  

            It doesn't clash with other definitions [https://confluence.lsstcorp.org/display/SYSENG/Image+naming+and+visit+definition+conventions] (K-T Lim's comment). 

             
            ksuberlak Krzysztof Suberlak made changes -
            Reviewers Robert Lupton [ rhl ] Kian-Tat Lim [ ktl ]
            Hide
            Permalink
            ktl Kian-Tat Lim added a comment -

            Looks reasonable. Thanks for adding the test.

            Show
            ktl Kian-Tat Lim added a comment - Looks reasonable. Thanks for adding the test.
            ktl Kian-Tat Lim made changes -
            Status In Review [ 10004 ] Reviewed [ 10101 ]
            ksuberlak Krzysztof Suberlak made changes -
            Resolution Done [ 10000 ]
            Status Reviewed [ 10101 ] Done [ 10002 ]
            ksuberlak Krzysztof Suberlak made changes -
            Link This issue is child task of DM-28556 [ DM-28556 ]
            ksuberlak Krzysztof Suberlak made changes -
            Epic Link DM-28335 [ 445220 ]
            Team DM Science [ 12218 ] Telescope and Site [ 13500 ]
            Labels SciencePipelines UW
            ksuberlak Krzysztof Suberlak made changes -
            Link This issue blocks DM-26836 [ DM-26836 ]
            ajc Andrew Connolly made changes -
            Story Points 4

              People

              Assignee:
              ksuberlak Krzysztof Suberlak
              Reporter:
              ksuberlak Krzysztof Suberlak
              Reviewers:
              Kian-Tat Lim
              Watchers:
              Kian-Tat Lim, Krzysztof Suberlak, Robert Lupton, Tim Jenness
              Votes:
              0 Vote for this issue
              Watchers:
              4 Start watching this issue

                Dates

                Created:
                Updated:
                Resolved:

                  Jenkins

                  No builds found.