Uploaded image for project: 'Request For Comments'
  1. Request For Comments
  2. RFC-544

ISR rewrites for reduced code duplication

    XMLWordPrintable

    Details

    • Type: RFC
    • Status: Implemented
    • Resolution: Done
    • Component/s: DM
    • Labels:
      None

      Description

      I've been reworking the `ip_isr` package to include the functionality that existed in `obs_subaru`, with the goal of reducing duplicate code.  Side benefits of this rework are that the ISR processing is clearer, more uniform across cameras, and easier to debug.  There are some API changes associated with this which I wanted to make clear before the ticket for this issue is reviewed and merged, to collect comments and concerns.  A quick summary of API changes:

      • `obs_subaru` and `obs_decam` no longer have separate ISR tasks.  The functionality has been folded into the standard `IsrTask`.
      • New subtasks exist to handle straylight, vignette construction, and camera-specific masking.
      • The methods `brighterFatterCorrection`, `gainContext`, `addDistortionModel` and `attachTransmissionCurve` are migrated from isrTask.py to isrFunctions.py, with the remainder of the ISR operations.  Methods left in IsrTask handle configuration options and iteration instead of pixel operations.
      • Many more configuration options in `IsrTaskConfig`.

      I used the `obs_subaru` package to define the sequence of ISR operations, which differed slightly from that in `ip_isr`.  The main changes are:

      • Crosstalk correction.  `ip_isr` placed this directly after overscan correction; `obs_subaru` waited until after bias subtraction and linearization.
      • Linearization. `ip_isr` performed this before generating the variance map; `obs_subaru` waited until after the variance map was constructed.

      The tests I've done show zero pixel difference for HSC, small deviations for `obs_lsstCam` cameras around saturated objects, and a slight variance offset for DECam due to the rearrangement of the processing order.

       

        Attachments

        1. decam_isr_flux_ratio.png
          decam_isr_flux_ratio.png
          10 kB
        2. flux_diff_matched.png
          flux_diff_matched.png
          8 kB
        3. snr_unmatched.png
          snr_unmatched.png
          7 kB
        4. source_compare.png
          source_compare.png
          111 kB
        5. unmatched-cyan_master-green_ticket.jpg
          unmatched-cyan_master-green_ticket.jpg
          479 kB

          Issue Links

            Activity

            Hide
            mrawls Meredith Rawls added a comment -

            Thanks all for taking a thorough look at the changes this will have for DECam in particular. I agree with Jim's summary and I believe the main result of adopting this RFC will be merging DM-15862. Let's do it!

            Show
            mrawls Meredith Rawls added a comment - Thanks all for taking a thorough look at the changes this will have for DECam in particular. I agree with Jim's summary and I believe the main result of adopting this RFC will be merging DM-15862 . Let's do it!
            Hide
            czw Christopher Waters added a comment -

            Great!  I'm running a final Jenkins test right now, so DM-15862 will likely hit master tomorrow afternoon.

            Show
            czw Christopher Waters added a comment - Great!  I'm running a final Jenkins test right now, so DM-15862 will likely hit master tomorrow afternoon.
            Hide
            swinbank John Swinbank added a comment -

            .

            Thanks everybody, and especially Christopher Waters!

            Show
            swinbank John Swinbank added a comment - . Thanks everybody, and especially Christopher Waters !
            Hide
            tjenness Tim Jenness added a comment -

            Christopher Waters it looks like you can mark this RFC as implemented.

            Show
            tjenness Tim Jenness added a comment - Christopher Waters it looks like you can mark this RFC as implemented.
            Hide
            rhl Robert Lupton added a comment -

            I'm most concerned about consistency between how calibrations are produced and how they are used.  It sounds like we can close the RFC – good!

            Show
            rhl Robert Lupton added a comment - I'm most concerned about consistency between how calibrations are produced and how they are used.  It sounds like we can close the RFC – good!

              People

              Assignee:
              czw Christopher Waters
              Reporter:
              czw Christopher Waters
              Watchers:
              Aaron Roodman, Christopher Waters, Colin Slater, Eli Rykoff, Jim Bosch, John Parejko, John Swinbank, Kian-Tat Lim, Meredith Rawls, Merlin Fisher-Levine, Robert Lupton, Tim Jenness, Yusra AlSayyad
              Votes:
              0 Vote for this issue
              Watchers:
              13 Start watching this issue

                Dates

                Created:
                Updated:
                Resolved:
                Planned End:

                  Jenkins

                  No builds found.