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

Update expBits used in gen2 bulter to match value computed for gen3

    XMLWordPrintable

    Details

    • Story Points:
      2
    • Epic Link:
    • Sprint:
      DRP S21b
    • Team:
      Data Release Production
    • Urgent?:
      No

      Description

      As was noted on DM-30812, there were differences between the gen2 and gen3 middleware src catalog outputs in the following columns:

      • id
      • parent
      • various ext_shapeHSM_HsmPsfMomentsDebiased_* columns
      • the total number of finite entries

      As Joshua Meyers pointed out on that ticket, all of these could stem from the id mismatch as it is used to seed a rng in the HsmPsfMomentsDebiased code. This ticket is to figure out where the mismatch is happening and sync them up (hopefully to the gen3 version) in hopes of achieving gen2/gen3 parity for the DC2 dataset.

        Attachments

          Issue Links

            Activity

            No builds found.
            lauren Lauren MacArthur created issue -
            lauren Lauren MacArthur made changes -
            Field Original Value New Value
            Link This issue relates to DM-30812 [ DM-30812 ]
            lauren Lauren MacArthur made changes -
            Link This issue relates to DM-30730 [ DM-30730 ]
            lauren Lauren MacArthur made changes -
            Status To Do [ 10001 ] In Progress [ 3 ]
            Hide
            lauren Lauren MacArthur added a comment -

            Indeed, if one goes in and looks at the values in the exposureIdInfo object in characterizeImage.run in pipe_tasks, the gen3 version has:

            exposureIdInfo.expBits = 34
            exposureIdInfo.expId = 496989094
            exposureIdInfo.maxBits = None
            exposureIdInfo.unusedBits = 29
            

            whereas the gen2 version has:

            exposureIdInfo.expBits = 38
            exposureIdInfo.expId = 496989094
            exposureIdInfo.maxBits = None
            exposureIdInfo.unusedBits = 25
            

            The gen2 number is being explicitly set in obs_lsst here, so here's hoping all that is required is to change that to 34 to match the gen3 version.

            Show
            lauren Lauren MacArthur added a comment - Indeed, if one goes in and looks at the values in the  exposureIdInfo object in characterizeImage.run in pipe_tasks , the gen3 version has: exposureIdInfo.expBits = 34 exposureIdInfo.expId = 496989094 exposureIdInfo.maxBits = None exposureIdInfo.unusedBits = 29 whereas the gen2 version has: exposureIdInfo.expBits = 38 exposureIdInfo.expId = 496989094 exposureIdInfo.maxBits = None exposureIdInfo.unusedBits = 25 The gen2 number is being explicitly set in obs_lsst here , so here's hoping all that is required is to change that to 34 to match the gen3 version.
            Hide
            lauren Lauren MacArthur added a comment -

            Ok, that seems to have done the trick. I ran singleFrameDriver on the entire DC2 list of visits with w_20201_25 this branch and, not only do the id's now match up with the gen3 versions, but the shapeHSM differences are now also gone (since the algorithm is now getting consistent rng seeds). The output of this run can be found in:

            /datasets/DC2/repoRun2.2i/rerun/w_2021_25/DM-30812/
            

            Show
            lauren Lauren MacArthur added a comment - Ok, that seems to have done the trick. I ran singleFrameDriver on the entire DC2 list of visits with w_20201_25 this branch and, not only do the id's now match up with the gen3 versions, but the shapeHSM differences are now also gone (since the algorithm is now getting consistent rng seeds). The output of this run can be found in: / datasets / DC2 / repoRun2. 2i / rerun / w_2021_25 / DM - 30812 /
            Hide
            lauren Lauren MacArthur added a comment -

            Jenkins is happy.

            Show
            lauren Lauren MacArthur added a comment - Jenkins is happy .
            Hide
            lauren Lauren MacArthur added a comment -

            Jim, as the original setter of this value, would you mind having a look and seeing if you are ok with this change?  (I tried to solicit a volunteer for this review, but was unsuccessful...)

            Show
            lauren Lauren MacArthur added a comment - Jim, as the original setter of this value, would you mind having a look and seeing if you are ok with this change?  (I tried to solicit a volunteer for this review, but was unsuccessful...)
            lauren Lauren MacArthur made changes -
            Reviewers James Chiang [ jchiang ]
            Status In Progress [ 3 ] In Review [ 10004 ]
            Hide
            jchiang James Chiang added a comment -

            Sure I can have a look.    However, I don't recall the exact context where this came up.  Can you point me at the original posting maybe?

            Show
            jchiang James Chiang added a comment - Sure I can have a look.    However, I don't recall the exact context where this came up.  Can you point me at the original posting maybe?
            Hide
            lauren Lauren MacArthur added a comment -

            Most of the details regarding the need for this change are in the ticket description and in DM-30812.  It boils down to our ambition to achieve exact parity between the gen2 & gen3 middlewares on the road to deprecating the former.  The way this value is derived in gen3, it gets set to 34.  As of your original commit (which looks like it was part of DM-18551), gen2 has this set to 38. This changes the object ids and, since ids are often used as predictable seeds for random number generators in some measurement algorithms, this can lead to a difference in the outputs. I guess my assumption here is that gen3 is setting the "correct" (i.e. that will be used for all future processing – see this community post for details) and I am just looking to match the two while gen2 is still in play (parity between the two is greatly desired, so any simple changes to make that happen are being made, even if it's an update to the soon-to-be-deprecated gen2 paths).  Does that make sense?

            Show
            lauren Lauren MacArthur added a comment - Most of the details regarding the need for this change are in the ticket description and in DM-30812 .  It boils down to our ambition to achieve exact parity between the gen2 & gen3 middlewares on the road to deprecating the former.  The way this value is derived in gen3, it gets set to 34.  As of your original commit (which looks like it was part of DM-18551 ), gen2 has this set to 38. This changes the object ids and, since ids are often used as predictable seeds for random number generators in some measurement algorithms, this can lead to a difference in the outputs. I guess my assumption here is that gen3 is setting the "correct" (i.e. that will be used for all future processing – see this community post for details) and I am just looking to match the two while gen2 is still in play (parity between the two is greatly desired, so any simple changes to make that happen are being made, even if it's an update to the soon-to-be-deprecated gen2 paths).  Does that make sense?
            Hide
            jchiang James Chiang added a comment -

            Yes, thank you! That is very helpful.  I had vague recollections of that commit of mine and of Tim's Community post, but couldn't recall exactly where to look (I somehow thought it discussed solely on slack.).  

            Show
            jchiang James Chiang added a comment - Yes, thank you! That is very helpful.  I had vague recollections of that commit of mine and of Tim's Community post, but couldn't recall exactly where to look (I somehow thought it discussed solely on slack.).  
            Hide
            jchiang James Chiang added a comment -

            The changes look good to me.  Based on the range of run numbers we used for DC2 and allowing for detector numbers up to 205, 34 bits should be fine. (I can't recall why I chose 38 last time...probably being overly conservative.)

            Show
            jchiang James Chiang added a comment - The changes look good to me.  Based on the range of run numbers we used for DC2 and allowing for detector numbers up to 205, 34 bits should be fine. (I can't recall why I chose 38 last time...probably being overly conservative.)
            Hide
            lauren Lauren MacArthur added a comment -

            Great, thanks Jim.  I just checked and no rebase is needed, so I think the original Jenkins run still holds.  When you're ready, please hit the "Review Complete" button (you could also officially "approve" the PR) and I will then go ahead and merge.

            Show
            lauren Lauren MacArthur added a comment - Great, thanks Jim.  I just checked and no rebase is needed, so I think the original Jenkins run still holds.  When you're ready, please hit the "Review Complete" button (you could also officially "approve" the PR) and I will then go ahead and merge.
            jchiang James Chiang made changes -
            Status In Review [ 10004 ] Reviewed [ 10101 ]
            lauren Lauren MacArthur made changes -
            Resolution Done [ 10000 ]
            Status Reviewed [ 10101 ] Done [ 10002 ]
            lauren Lauren MacArthur made changes -
            Story Points 2
            Hide
            lauren Lauren MacArthur added a comment -

            Thanks for the speedy review. Merged & done.

            Show
            lauren Lauren MacArthur added a comment - Thanks for the speedy review. Merged & done.

              People

              Assignee:
              lauren Lauren MacArthur
              Reporter:
              lauren Lauren MacArthur
              Reviewers:
              James Chiang
              Watchers:
              James Chiang, Jim Bosch, Lauren MacArthur
              Votes:
              0 Vote for this issue
              Watchers:
              3 Start watching this issue

                Dates

                Created:
                Updated:
                Resolved:

                  Jenkins

                  No builds found.