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

Dimension packing/unpacking broken for DECam

    XMLWordPrintable

    Details

    • Story Points:
      1
    • Epic Link:
    • Team:
      Data Release Production
    • Urgent?:
      No

      Description

      Jim kindly gave a handy code snippet which lets one retrieve a dataId (containing visit and detector information) if one has a butler, a known instrument, and what I'll call a `ccdVisitId` (the thing you get when you do visitId.getExposureId(), on a good day).

      The snippet is

      instrumentDataId = butler.registry.expandDataId(instrument=instrumentName)
      		 
      packer = butler.registry.dimensions.makePacker("visit_detector", instrumentDataId)
      		 
      dataId = packer.unpack(integerVisitDetectorId)
      		 
      assert packer.pack(dataId) == integerVisitDetectorId

      It works great for all the HSC cases I've tried, but does not work for DECam. Ideally it would work for any/all instruments.

      I tested it with `/project/mrawls/hits2015-3` and instrumentName = 'DECam' and some `ccdVisitId`s like 42160439 and 41125539, which are (conveniently) the visit and detector smashed together - 421604, 39 and 411255, 39 respectively. It returns things that aren't that.

        Attachments

          Issue Links

          relates to

          Story - Created by Jira Software - do not edit or delete. Issue type for a user story. DM-21856 Unify mangling between data ID packers and astro_metadata_translator

          • To Do

          Epic - Created by Jira Software - do not edit or delete. Issue type for a big user story that needs to be broken down. DM-13942 Remove exposureId and from VisitInfo and add visitId

          • Invalid

            Activity

            Ascending order - Click to sort in descending order
            9 older comments
            Hide
            Permalink
            mrawls Meredith Rawls added a comment -

            Hmm, I set up your obs_decam branch and tried the packer on my DECam repo (/project/mrawls/hits2015-3), and it didn't work.

            In [1]: import lsst.daf.butler as dafButler
            		 
            In [2]: butler = dafButler.Butler('/project/mrawls/hits2015-3')
            		 
            In [3]: instrumentDataId = butler.registry.expandDataId(instrument='DECam')
            		 
            In [4]: packer = butler.registry.dimensions.makePacker("visit_detector", instrumentDataId)
            		 
            In [5]: dataId = packer.unpack(41091533)
            		 
            In [6]: dataId
            		 
            Out[6]: {instrument: 'DECam', detector: 13, visit: 642055}
            		 
            In [7]: packer.unpack(41125539)
            		 
            Out[7]: {instrument: 'DECam', detector: 35, visit: 642586}

            Show
            mrawls Meredith Rawls added a comment - Hmm, I set up your obs_decam branch and tried the packer on my DECam repo (/project/mrawls/hits2015-3), and it didn't work. In [ 1 ]: import lsst.daf.butler as dafButler In [ 2 ]: butler = dafButler.Butler( '/project/mrawls/hits2015-3' ) In [ 3 ]: instrumentDataId = butler.registry.expandDataId(instrument= 'DECam' ) In [ 4 ]: packer = butler.registry.dimensions.makePacker( "visit_detector" , instrumentDataId) In [ 5 ]: dataId = packer.unpack( 41091533 ) In [ 6 ]: dataId Out[ 6 ]: {instrument: 'DECam' , detector: 13 , visit: 642055 } In [ 7 ]: packer.unpack( 41125539 ) Out[ 7 ]: {instrument: 'DECam' , detector: 35 , visit: 642586 }
            Hide
            Permalink
            jbosch Jim Bosch added a comment -

            Unfortunately this will require new repos to take effect (part of the reason finally got around to this now is that I'm putting together the long-lived shared repos now, and new it was something I needed to do before registering DECam).

            But if you've got a personal repo you want to patch, you can fire up SQLite on your gen3.sqlite3 file, and do 

            UPDATE instrument SET detector_max=100 WHERE name='DECam';

            This is one of those rare cases where I think the direct-SQL patch is really not too bad. If you want it done in a shared repo, I'm happy to take care of it if that SQL patch makes you nervous.

            Show
            jbosch Jim Bosch added a comment - Unfortunately this will require new repos to take effect (part of the reason finally got around to this now is that I'm putting together the long-lived shared repos now, and new it was something I needed to do before registering DECam). But if you've got a personal repo you want to patch, you can fire up SQLite on your gen3.sqlite3 file, and do UPDATE instrument SET detector_max=100 WHERE name='DECam'; This is one of those rare cases where I think the direct-SQL patch is really not too bad. If you want it done in a shared repo, I'm happy to take care of it if that SQL patch makes you nervous.
            Hide
            Permalink
            mrawls Meredith Rawls added a comment -

            Aha, thanks, that did the trick! I think the code changes look fine.

            Show
            mrawls Meredith Rawls added a comment - Aha, thanks, that did the trick! I think the code changes look fine.
            Hide
            Permalink
            jbosch Jim Bosch added a comment -

            Jenkins politely reminded me that running the obs_decam tests without testdata_decam setup isn't terribly useful, and that revealed that I had some problems on the branch you reviewed. They were all pretty straightforward to fix, so it's up to you if you want to take another look. And if you want any existing repos to reflect these changes you'll also want to run:

            UPDATE instrument SET visit_max=33554432 WHERE name='DECam';
            		 
            UPDATE instrument SET exposure_max=33554432 WHERE name='DECam';

            Show
            jbosch Jim Bosch added a comment - Jenkins politely reminded me that running the obs_decam tests without testdata_decam setup isn't terribly useful, and that revealed that I had some problems on the branch you reviewed. They were all pretty straightforward to fix, so it's up to you if you want to take another look. And if you want any existing repos to reflect these changes you'll also want to run: UPDATE instrument SET visit_max=33554432 WHERE name='DECam'; UPDATE instrument SET exposure_max=33554432 WHERE name='DECam';
            Hide
            Permalink
            mrawls Meredith Rawls added a comment -

            Ack, I'm glad you caught that. Assuming Jenkins is happy with testdata_decam, I'm happy too. Thanks for the additional sqlite lines!

            Show
            mrawls Meredith Rawls added a comment - Ack, I'm glad you caught that. Assuming Jenkins is happy with testdata_decam , I'm happy too. Thanks for the additional sqlite lines!

              People

              Assignee:
              jbosch Jim Bosch
              Reporter:
              mrawls Meredith Rawls
              Reviewers:
              Meredith Rawls
              Watchers:
              Jim Bosch, John Parejko, Meredith Rawls, Tim Jenness, Yusra AlSayyad
              Votes:
              0 Vote for this issue
              Watchers:
              5 Start watching this issue

                Dates

                Created:
                Updated:
                Resolved:

                  Jenkins

                  No builds found.