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

Save Detectors with master calibrations

    XMLWordPrintable

    Details

    • Type: Story
    • Status: Done
    • Resolution: Done
    • Fix Version/s: None
    • Component/s: None
    • Labels:
      None
    • Story Points:
      2
    • Team:
      Data Release Production
    • Urgent?:
      No

      Description

      It's looking like the interface to DM-29732 could be a lot cleaner if e.g. a master bias was saved with its own Detector.  That's because in order to get an amp subimage, one has to provide a Detector reflecting the full bias image to the code extracting the subimage, as well as an amp describing the subimage to extract, for two reasons:

      • we need an existing Detector to make a single-amp Detector to return with the subimage (for all of the detector-wide information that doesn't change when it's restricted to one amp);
      • it's possible for bias (and maybe some niche derived-from-raw datasets) to be trimmed or untrimmed, and looking at Detector.getBBox() (always trimmed) vs. the on-disk image bbox seems to be the best way to figure that out.

      But right now it seems pretty difficult to get the appropriate Detector for a bias (even absent that question about whether it's trimmed or not):

      butler.get("camera", ...)[detectorId] 

      will get you the wrong amplifiers for LSST (reflecting the unassembled on-disk raws),

      butler.get("bias", ...).getDetector()
      # and
      butler.get("bias.detector, ...)

      are None,

      and

       butler.get("raw.detector", ...)

      doesn't work at all.  I'm going to fix the latter on DM-29370 (already on the branches), because it's something we should have been supporting before, and so we'll at least have something.

      But that's an awkward way to get a detector for some other image, and for LATISS (at least), you can still get something that doesn't match an untrimmed bias because of the raw overscan patching that can happen - but I think that shouldn't be a problem in practice.

      Anyhow, the easy way to fix this is to just propagate a Detector through the CPP combine tasks and save it with the Exposure.

        Attachments

          Issue Links

            Activity

            Hide
            czw Christopher Waters added a comment -

            The unit test coverage on cp_pipe needs to be updated, or moved to ci_cpp_gen3.  In any case, jenkins succeeded (https://ci.lsst.codes/blue/organizations/jenkins/stack-os-matrix/detail/stack-os-matrix/34124/pipeline/), and a quick command line check indicates the issue seems to be fixed.

            Show
            czw Christopher Waters added a comment - The unit test coverage on cp_pipe needs to be updated, or moved to ci_cpp_gen3 .  In any case, jenkins succeeded ( https://ci.lsst.codes/blue/organizations/jenkins/stack-os-matrix/detail/stack-os-matrix/34124/pipeline/),  and a quick command line check indicates the issue seems to be fixed.
            Hide
            tjenness Tim Jenness added a comment -

            The lines look okay but it would be nice if we could have a test somewhere that showed the detector in the combined bias matches the detector in the input. I assume you check elsewhere that all the detectors match in each of the inputs?

            Show
            tjenness Tim Jenness added a comment - The lines look okay but it would be nice if we could have a test somewhere that showed the detector in the combined bias matches the detector in the input. I assume you check elsewhere that all the detectors match in each of the inputs?
            Hide
            czw Christopher Waters added a comment -

            I've added a check that all the detectors match, which I'd been relying on butler to handle previously.  I think that this check ensures the first will be true, but doesn't test it.

            Show
            czw Christopher Waters added a comment - I've added a check that all the detectors match, which I'd been relying on butler to handle previously.  I think that this check ensures the first will be true, but doesn't test it.
            Hide
            tjenness Tim Jenness added a comment -

            Minor comments on PR.

            Show
            tjenness Tim Jenness added a comment - Minor comments on PR.
            Hide
            tjenness Tim Jenness added a comment -

            Do you have any tests that combine some data into a bias that you can then check comes back with a detector attached?

            Show
            tjenness Tim Jenness added a comment - Do you have any tests that combine some data into a bias that you can then check comes back with a detector attached?
            Hide
            tjenness Tim Jenness added a comment -

            I finally saw your comment about lack of tests. Okay.

            Show
            tjenness Tim Jenness added a comment - I finally saw your comment about lack of tests. Okay.

              People

              Assignee:
              czw Christopher Waters
              Reporter:
              jbosch Jim Bosch
              Reviewers:
              Tim Jenness
              Watchers:
              Christopher Waters, Jim Bosch, Robert Lupton, Tim Jenness
              Votes:
              0 Vote for this issue
              Watchers:
              4 Start watching this issue

                Dates

                Created:
                Updated:
                Resolved:

                  Jenkins

                  No builds found.