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

obs_lsst accounts for detectors twice in IDs

    XMLWordPrintable

    Details

    • Type: Bug
    • Status: Done
    • Resolution: Done
    • Fix Version/s: None
    • Component/s: obs_lsst
    • Labels:
      None
    • Team:
      Architecture
    • Urgent?:
      No

      Description

      https://github.com/lsst/obs_lsst/blob/master/python/lsst/obs/lsst/_instrument.py#L117-L128 uses the max_detector_exposure_id as the maximum value of the visit and observation dimensions. But those maximums are later multiplied again by the maximum detector count. I believe the max_exposure_id should be used instead.

        Attachments

          Activity

          Hide
          ktl Kian-Tat Lim added a comment -

          A one-word change. Passes Jenkins on macOS; I don't anticipate problems on CentOS.

          Show
          ktl Kian-Tat Lim added a comment - A one-word change. Passes Jenkins on macOS; I don't anticipate problems on CentOS.
          Hide
          jbosch Jim Bosch added a comment -

          Looks good. We will have to create new repos to make sure of this change, and I am a bit worried that exposure_max == visit_max remains a bit naive. But this is certainly a step in the right direction, and as we discussed on slack, we might need to shrink these anyway if we want Source IDs to fit in 64 bits.

          Show
          jbosch Jim Bosch added a comment - Looks good. We will have to create new repos to make sure of this change, and I am a bit worried that exposure_max == visit_max remains a bit naive. But this is certainly a step in the right direction, and as we discussed on slack, we might need to shrink these anyway if we want Source IDs to fit in 64 bits.
          Hide
          ktl Kian-Tat Lim added a comment -

          I didn't think about backwards compatibility. Using this code with old repos could cause problems decoding existing IDs, even if new ones are OK? That would mean that this is actually an interface-breaking change.

          Show
          ktl Kian-Tat Lim added a comment - I didn't think about backwards compatibility. Using this code with old repos could cause problems decoding existing IDs, even if new ones are OK? That would mean that this is actually an interface-breaking change.
          Hide
          jbosch Jim Bosch added a comment -

          This should be completely safe to use with old repos. The bug will just not be fixed in old repos, and the packed IDs generated by code run against old repos will be inconsistent with the packed IDs generated by code run against new repos.

          Show
          jbosch Jim Bosch added a comment - This should be completely safe to use with old repos. The bug will just not be fixed in old repos, and the packed IDs generated by code run against old repos will be inconsistent with the packed IDs generated by code run against new repos.

            People

            Assignee:
            ktl Kian-Tat Lim
            Reporter:
            ktl Kian-Tat Lim
            Reviewers:
            Jim Bosch
            Watchers:
            Jim Bosch, Kian-Tat Lim, Tim Jenness
            Votes:
            0 Vote for this issue
            Watchers:
            3 Start watching this issue

              Dates

              Created:
              Updated:
              Resolved:

                Jenkins Builds

                No builds found.