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

Aperture correction field keys not guaranteed to point the same offsets within a given reprocessing

    Details

    • Type: Bug
    • Status: Done
    • Resolution: Done
    • Fix Version/s: None
    • Component/s: meas_base
    • Labels:
      None
    • Story Points:
      0.5
    • Sprint:
      DRP F18-5
    • Team:
      Data Release Production

      Description

      In perusing the pipe_analysis outputs of the RC2 reprocessing runs to check that our results are all looking as they should, I noticed that the aperture corrections were comparing differently between reprocessing runs (e.g. w42 vs. w41). While, in principle, this could happen as a result of code changes, this did not appear to be the case as the flux comparisons were all identical. Digging into it, I discovered that the key offsets point to a different place for different patches/visits for all aperture correction fields within the same processing run. I.e. the catalog schemas don't match each other, nor the one defined in the schema files in the repo.

      E.g. in /datasets/hsc/repo/rerun/RC/w_2018_42/DM-16095/, the meas cat schema in schema/deepCoadd_meas.fits has:

      base_GaussianFlux_apCorr Key<D>(offset=1832, nElements=1)
      base_PsfFlux_apCorr Key<D>(offset=1992, nElements=1)
      

      But for two patches, we find:

      dataId = {"patch": "8,0", "tract": 9615, "filter":"HSC-I"}
      base_GaussianFlux_apCorr Key<D>(offset=2040, nElements=1)
      base_PsfFlux_apCorr Key<D>(offset=1992, nElements=1)
      

      dataId = {"patch": "3,7", "tract": 9615, "filter":"HSC-I"}
      base_GaussianFlux_apCorr Key<D>(offset=1880, nElements=1)
      base_PsfFlux_apCorr Key<D>(offset=1928, nElements=1)
      

      This is most undesirable as it precludes concatenating all patches in one tract into a single catalog (which requires them all to have identical schemas).  The fix may be as simple as looping over a sorted list (as opposed to one where order is not guaranteed) when adding the apCorr keys to the schema.

        Attachments

          Activity

          Hide
          jbosch Jim Bosch added a comment -

          I think I've spotted it: iteration over a set in meas_base.

          Show
          jbosch Jim Bosch added a comment - I think I've spotted it: iteration over a set in meas_base.
          Hide
          jbosch Jim Bosch added a comment -

          Lauren MacArthur, I think this should fix it.  I'm inclined to just say we should merge it after review and let the final testing in pipe_analysis wait for the next RC run, rather than going to any extra effort before merging.  The cause-and-effect is pretty clear, and at worst we're fixing one bug and will uncover another.

          Show
          jbosch Jim Bosch added a comment - Lauren MacArthur , I think this should fix it.  I'm inclined to just say we should merge it after review and let the final testing in pipe_analysis wait for the next RC run, rather than going to any extra effort before merging.  The cause-and-effect is pretty clear, and at worst we're fixing one bug and will uncover another.
          Hide
          lauren Lauren MacArthur added a comment -

          Agreed. If Jenkins is happy, I’m happy

          Show
          lauren Lauren MacArthur added a comment - Agreed. If Jenkins is happy, I’m happy
          Hide
          jbosch Jim Bosch added a comment - - edited

          Re-running Jenkins (https://ci.lsst.codes/blue/organizations/jenkins/stack-os-matrix/detail/stack-os-matrix/29001/pipeline) since the first time I got unlucky and kicked it off when master was broken.  Lauren MacArthur, ping me if I seem to have forgotten about this later today; I'd like to get it into the weekly, and I'm sure you would, too.

          Show
          jbosch Jim Bosch added a comment - - edited Re-running Jenkins ( https://ci.lsst.codes/blue/organizations/jenkins/stack-os-matrix/detail/stack-os-matrix/29001/pipeline ) since the first time I got unlucky and kicked it off when master was broken.  Lauren MacArthur , ping me if I seem to have forgotten about this later today; I'd like to get it into the weekly, and I'm sure you would, too.
          Hide
          lauren Lauren MacArthur added a comment -

          Will do!

          Show
          lauren Lauren MacArthur added a comment - Will do!
          Hide
          lauren Lauren MacArthur added a comment -

          Looks like Jenkins just gave you the green light

          Show
          lauren Lauren MacArthur added a comment - Looks like Jenkins just gave you the green light
          Hide
          jbosch Jim Bosch added a comment -

          Merged to master.

          Show
          jbosch Jim Bosch added a comment - Merged to master.

            People

            • Assignee:
              jbosch Jim Bosch
              Reporter:
              lauren Lauren MacArthur
              Reviewers:
              Lauren MacArthur
              Watchers:
              Jim Bosch, John Swinbank, Lauren MacArthur, Yusra AlSayyad
            • Votes:
              0 Vote for this issue
              Watchers:
              4 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Summary Panel