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

Unify mangling between data ID packers and astro_metadata_translator

    Details

    • Story Points:
      2
    • Team:
      Architecture

      Description

      astro_metadata_translator and the data ID packers in daf_butler (which may move to obs_base on DM-21855) currently duplicate logic that mangles certain data IDs into single integers.  Various downstream code probably assumes that logic is identical, which means this is a fragility problem.

      I can think of a few ways to fix this:

      • If the mangled ID in astro_metadata_translator is present just to provide an ID to afw.image.VisitInfo, we might want to consider deprecating and removing that field from VisitInfo, and dropping the logic from astro_metadata_translator (I think we might want to drop it from VisitInfo regardless - it's the only place one of our data product files records anything about its own data ID, and I'm not convinced we need it).
      • Move the actual implementation the mangling in astro_metadata_translator to a separate, public, free function or class so the DimensionPacker in daf_butler/obs_base can call it.

       

        Attachments

          Activity

          Hide
          tjenness Tim Jenness added a comment -

          In astro_metadata_translator the detector_exposure_id is the equivalent of the VisitInfo exposureId (and not the exposure_id which is something else in astro_metadata_translator). VisitInfo serializes this number to an EXPID header in output files.

          When makeRawVisitInfo is called with an exposureId that parameter is ignored by HSC, LSST cameras, and DECam since it is assumed that astro_metadata_translator will work it out. In gen2 exposureId is calculated from _computeCcdExposureId and passed to makeRawVisitInfo and then ignored by the cameras that use ObservationInfo.

          ap_association, cp_pipe, dax_apdb, ip_isr all use visitInfo.getExposureId directly.
          meas_base defines a version that uses a dataRef to get the exposure Id from butler (and there is one routine in pipe_tasks that also does that).

          In astro_metadata_translator compute_detector_exposure_id is a general routine used by obs_lsst and shared by gen2 and gen3. LATISS has a special-cased implementation where detector_exposure_id == exposure_id because there is only one detector (I think we can probably make that generic by trapping a max_detector=1).

          I think in the current implementation the only places that use the astro_metadata_translator detector_exposure_id are:

          • The gen2 obs_lsst Mapper (So that it's guaranteed to agree with gen3)
          • VisitInfo makeRawVisitInfo

          Gen2 removal helps clean up a lot of uncertainty. The VisitInfo is a harder question. Removing exposureId obviously helps a lot but if we have to keep it then, for example, the FitsRawFormatter in gen3 has to be able to calculate the detector_exposure_id itself to store it in the VisitInfo that is returned as one of the components. The formatter doesn't really have a means of asking butler for the detector_exposure_id at the moment but it does have the dataId available so in theory the gen3 Packer could be part of the public interface and used by the Formatter.

          I think that the following options work:

          • Remove detector_exposure_id from ObservationInfo and VisitInfo
          • Keep it in VisitInfo but remove from ObservationInfo. Make butler function available to convert a dataId to a detector_exposure_id for Formatters to use to populate VisitInfo.

          Either way I think once gen2 disappears and we really are using the gen3 packers, then we can remove it from ObservationInfo (but I think we need to retain the special logic for max_detectors=1 just returning the exposure_id). The only argument in favor of retaining it in astro_metadata_translators and moving the butler Packers there is that it lets people see the answer easily when they run translate_header command line tool.

          Show
          tjenness Tim Jenness added a comment - In astro_metadata_translator the detector_exposure_id is the equivalent of the VisitInfo exposureId (and not the exposure_id which is something else in astro_metadata_translator). VisitInfo serializes this number to an EXPID header in output files. When makeRawVisitInfo is called with an exposureId that parameter is ignored by HSC, LSST cameras, and DECam since it is assumed that astro_metadata_translator will work it out. In gen2 exposureId is calculated from _computeCcdExposureId and passed to makeRawVisitInfo and then ignored by the cameras that use ObservationInfo. ap_association, cp_pipe, dax_apdb, ip_isr all use visitInfo.getExposureId directly. meas_base defines a version that uses a dataRef to get the exposure Id from butler (and there is one routine in pipe_tasks that also does that). In astro_metadata_translator compute_detector_exposure_id is a general routine used by obs_lsst and shared by gen2 and gen3. LATISS has a special-cased implementation where detector_exposure_id == exposure_id because there is only one detector (I think we can probably make that generic by trapping a max_detector=1). I think in the current implementation the only places that use the astro_metadata_translator detector_exposure_id are: The gen2 obs_lsst Mapper (So that it's guaranteed to agree with gen3) VisitInfo makeRawVisitInfo Gen2 removal helps clean up a lot of uncertainty. The VisitInfo is a harder question. Removing exposureId obviously helps a lot but if we have to keep it then, for example, the FitsRawFormatter in gen3 has to be able to calculate the detector_exposure_id itself to store it in the VisitInfo that is returned as one of the components. The formatter doesn't really have a means of asking butler for the detector_exposure_id at the moment but it does have the dataId available so in theory the gen3 Packer could be part of the public interface and used by the Formatter. I think that the following options work: Remove detector_exposure_id from ObservationInfo and VisitInfo Keep it in VisitInfo but remove from ObservationInfo. Make butler function available to convert a dataId to a detector_exposure_id for Formatters to use to populate VisitInfo. Either way I think once gen2 disappears and we really are using the gen3 packers, then we can remove it from ObservationInfo (but I think we need to retain the special logic for max_detectors=1 just returning the exposure_id). The only argument in favor of retaining it in astro_metadata_translators and moving the butler Packers there is that it lets people see the answer easily when they run translate_header command line tool.

            People

            • Assignee:
              tjenness Tim Jenness
              Reporter:
              jbosch Jim Bosch
              Watchers:
              Jim Bosch, Tim Jenness
            • Votes:
              0 Vote for this issue
              Watchers:
              2 Start watching this issue

              Dates

              • Created:
                Updated:

                Summary Panel