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

Exposure id info bit counts expect unsigned ids but need to fit into signed

    XMLWordPrintable

    Details

    • Urgent?:
      No

      Description

      Jennifer Pollack reported the following error:

      File “src/table/IdFactory.cc”, line 57, in lsst::afw::table::{anonymous}::SourceIdFactory::SourceIdFactory(lsst::afw::table::RecordId, int)Exposure ID ‘3014091200001153’ is too large. {0}lsst::pex::exceptions::InvalidParameterError: ’Exposure ID ‘3014091200001153’ is too large.’
      

      This seemed strange because it fits into the declared 52 bits from https://github.com/lsst/obs_lsst/blob/master/python/lsst/obs/lsst/lsstCamMapper.py#L248

      It turns out that the code that checks the size of the id, https://github.com/lsst/afw/blob/master/src/table/IdFactory.cc#L52-L60, is using an int64_t https://github.com/lsst/afw/blob/master/include/lsst/afw/table/misc.h#L22

      As a result, any ids that actually extend into the 52nd bit trigger the error.

      If converting to a uint64_t is actually as difficult as claimed in the comments because of FITS, that may unexpectedly constrain us in an area where we were already running out of bits.

      In the meantime, it seems the best solution is to properly specify the maximum exposure id length as 63 bits in https://github.com/lsst/obs_base/blob/master/python/lsst/obs/base/exposureIdInfo.py#L58. It appears that no one attempts to override this in the obs packages.

      I am not certain if the dimension packer in daf_butler is also implicated due to https://github.com/lsst/pipe_tasks/blob/master/python/lsst/pipe/tasks/characterizeImage.py#L338-L339

        Attachments

          Issue Links

            Activity

            Hide
            tjenness Tim Jenness added a comment -

            We never expected controller=C data to be used with processCcd because it can't ever be used to give on-sky images.

            Of course phosim is going to be even worse.

            I thought there was a ticket to revamp source ID calculations completely to make them less dependent on direct use of the detector_exposure_id but I haven't found the ticket yet.

            The original discussion on community https://community.lsst.org/t/calculating-exposure-ids-for-obs-lsst-cameras-and-beyond/3572/26 made clear that we were assuming that this would be solved before real on-sky data turned up.

            Show
            tjenness Tim Jenness added a comment - We never expected controller=C data to be used with processCcd because it can't ever be used to give on-sky images. Of course phosim is going to be even worse. I thought there was a ticket to revamp source ID calculations completely to make them less dependent on direct use of the detector_exposure_id but I haven't found the ticket yet. The original discussion on community https://community.lsst.org/t/calculating-exposure-ids-for-obs-lsst-cameras-and-beyond/3572/26 made clear that we were assuming that this would be solved before real on-sky data turned up.
            Hide
            jbosch Jim Bosch added a comment -

            I think DM-17689 (which I'm now finally about to finish) will take care of the near-term problem, in that the 64s will be replaced with 63s, and will cease to be magic numbers. I think it'll be appropriate to close this ticket when that one finishes, then, as I think we've got other ones (or should have) for dealing with the harder problem how to make everything fit in the number of bits we have.

            Show
            jbosch Jim Bosch added a comment - I think DM-17689 (which I'm now finally about to finish) will take care of the near-term problem, in that the 64s will be replaced with 63s, and will cease to be magic numbers. I think it'll be appropriate to close this ticket when that one finishes, then, as I think we've got other ones (or should have) for dealing with the harder problem how to make everything fit in the number of bits we have.

              People

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

                Dates

                Created:
                Updated:

                  CI Builds

                  No builds found.