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

Finishing writing the basic version of the PTC task

    Details

    • Type: Story
    • Status: Done
    • Resolution: Done
    • Fix Version/s: None
    • Component/s: cp_pipe
    • Labels:
      None

      Description

      The stack infrastructure work is done already (brought to this ticket, used to live on DM-19709), this ticket is for Andrés Alejandro Plazas Malagón to use this task outline to actually produce a naive PTC curve.

      DM-20070 exists to enhance this task, bringing in the fancy algorithmic techniques provided by Pierre^2.

      Naive PTC task:

      • Main PTC plot with cubic fit and the linear components displayed for the gain
      • Mean vs exp. time (so that relative QE can be compared across runs)
      • This will need augmentation later to allow correcting with the photocharge on the monitoring diode
      • Read noise from var axis intercept
      • Persist all the relevant data products from the above plots as pickled dicts

      For later, we will want several modes of operation: [covariance summation, binning, ramp-frames?] and perhaps others, but these probably belong on the enhancement ticket, DM-20070.

      Binning as default, and the only supported mode for now.

      It is accepted that the linearity model calculation part of this will likely output garbage for now, as the dominant errors will likely be from:

      • Shutter variation (commanded vs. delivered)
      • Illumination/effective-expTime non-uniformity due to iris shutter itself
      • Lamp variability
        as all the above will need to be corrected for using photodiode data in order to get good (or likely even useful IMO) results, and this will require a post hoc supplementary analysis for now, as any kind of automated retrieval and processing of this data is clearly overambitious at this stage.

        Attachments

          Issue Links

            Activity

            Hide
            Parejkoj John Parejko added a comment -

            Should there also be PRs for the obs_base and obs_lsst changes?

            Show
            Parejkoj John Parejko added a comment - Should there also be PRs for the obs_base and obs_lsst changes?
            Hide
            swinbank John Swinbank added a comment -

            I understand from chatting to John (and from looking at https://github.com/lsst/cp_pipe/pull/13#pullrequestreview-258549539) that he's busy reviewing defects-finding code. I worry that that's a separate ticket (DM-20068) which isn't ready for review yet.

            Andrés Alejandro Plazas Malagón, can you confirm what actually needs reviewing here? Is the defects stuff really ready to go, or is it included in this work by mistake?

            John Parejko, per our discussion earlier — I really recommend holding off on this review until the above is clear.

            Show
            swinbank John Swinbank added a comment - I understand from chatting to John (and from looking at https://github.com/lsst/cp_pipe/pull/13#pullrequestreview-258549539 ) that he's busy reviewing defects-finding code. I worry that that's a separate ticket ( DM-20068 ) which isn't ready for review yet. Andrés Alejandro Plazas Malagón , can you confirm what actually needs reviewing here? Is the defects stuff really ready to go, or is it included in this work by mistake? John Parejko , per our discussion earlier — I really recommend holding off on this review until the above is clear.
            Hide
            plazas Andrés Alejandro Plazas Malagón added a comment -

            The reviewed I asked John Parejko to do is for this ticket about implementing code to calculate the Photon Transfer Curve (DM-20069). Reviewing the defects code is a separate thing, as ticketed in DM-DM-20068

            Show
            plazas Andrés Alejandro Plazas Malagón added a comment - The reviewed I asked John Parejko to do is for this ticket about implementing code to calculate the Photon Transfer Curve ( DM-20069 ). Reviewing the defects code is a separate thing, as ticketed in DM- DM-20068 . 
            Hide
            Parejkoj John Parejko added a comment -

            That was not made clear to me. The defects code is on the same PR.

            Show
            Parejkoj John Parejko added a comment - That was not made clear to me. The defects code is on the same PR.
            Hide
            plazas Andrés Alejandro Plazas Malagón added a comment -

            I apologize for any confusion. John Swinbank: should the defects code be reviewed along with the PTC code? The former seems ready to be reviewed too. 

            Show
            plazas Andrés Alejandro Plazas Malagón added a comment - I apologize for any confusion. John Swinbank : should the defects code be reviewed along with the PTC code? The former seems ready to be reviewed too. 
            Hide
            swinbank John Swinbank added a comment -

            Looking at the code, it doesn't seem to me that the defects task is ready for review (for example, it still contains a bunch of TODO comments). I suggest splitting the PTC code off into a different branch, so that John can review that separately.

            Show
            swinbank John Swinbank added a comment - Looking at the code, it doesn't seem to me that the defects task is ready for review (for example, it still contains a bunch of TODO comments). I suggest splitting the PTC code off into a different branch, so that John can review that separately.
            Hide
            swinbank John Swinbank added a comment -

            Pulled this out of review pending DM-20068. Apologies for the confusion John Parejko!

            Show
            swinbank John Swinbank added a comment - Pulled this out of review pending DM-20068 . Apologies for the confusion John Parejko !
            Hide
            czw Christopher Waters added a comment -

            I think I'm generally happy about this set of changes, but if the issues I was confused on (listed mostly in the `cp_pipe` PR) could be clarified, that would be great.

            Show
            czw Christopher Waters added a comment - I think I'm generally happy about this set of changes, but if the issues I was confused on (listed mostly in the `cp_pipe` PR) could be clarified, that would be great.

              People

              • Assignee:
                plazas Andrés Alejandro Plazas Malagón
                Reporter:
                mfisherlevine Merlin Fisher-Levine
                Reviewers:
                Christopher Waters
                Watchers:
                Andrés Alejandro Plazas Malagón, Andrew Bradshaw, Christopher Waters, John Parejko, John Swinbank, Merlin Fisher-Levine
              • Votes:
                0 Vote for this issue
                Watchers:
                6 Start watching this issue

                Dates

                • Created:
                  Updated:
                  Resolved:

                  Summary Panel