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

Understand and ensure variance plane compliance with diffim decorrelation

    XMLWordPrintable

    Details

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

      Description

      Understand how the variance plane should be adjusted in the decorrelation (ZOGY) correction, and ensure it is being done correctly.

        Attachments

          Issue Links

            Activity

            Hide
            reiss David Reiss added a comment -

            Work underway. Additional test cases added to testImageDecorrelation.py - so this will have an IPython notebook as well as additional code in ip_diffim.

            Show
            reiss David Reiss added a comment - Work underway. Additional test cases added to testImageDecorrelation.py - so this will have an IPython notebook as well as additional code in ip_diffim.
            Hide
            reiss David Reiss added a comment - - edited

            It appears that the convolution does handle variance plane correctly, and the variance is scaled appropriately back to (nearly) the theoretical value. This may be summarized in this IPython notebook and specifically this figure, which shows histograms of the per-pixel variances for an imageDifference run on a single DECam CCD, including the two original images (science and template), the "uncorrected" diffim ("orig"), the decorrelated diffim ("corr", for corrected) and the naive expected variances which is the sum of the raw variances ("expected"):

            The mean values of the corrected and expected variances differ by about 0.5-sigma. The 1-sigma spread in variances is greater in the corrected diffim than one would naively expect. This is likely due to issues being investigated in DM-6243 (the fact that we use a single decorrelation kernel based on matching kernel computed from the center of the image, and based on single (scalar) variances for the two input images). It is not clear that this will present an issue (again, see DM-6243).

            An additional analysis that is relevant to the variances is that of the resulting detections and their measured SNRs. This is to be part of DM-6244.

            This ticket is now considered Done.

            Show
            reiss David Reiss added a comment - - edited It appears that the convolution does handle variance plane correctly, and the variance is scaled appropriately back to (nearly) the theoretical value. This may be summarized in this IPython notebook and specifically this figure, which shows histograms of the per-pixel variances for an imageDifference run on a single DECam CCD, including the two original images (science and template), the "uncorrected" diffim ("orig"), the decorrelated diffim ("corr", for corrected) and the naive expected variances which is the sum of the raw variances ("expected"): The mean values of the corrected and expected variances differ by about 0.5-sigma. The 1-sigma spread in variances is greater in the corrected diffim than one would naively expect. This is likely due to issues being investigated in DM-6243 (the fact that we use a single decorrelation kernel based on matching kernel computed from the center of the image, and based on single (scalar) variances for the two input images). It is not clear that this will present an issue (again, see DM-6243 ). An additional analysis that is relevant to the variances is that of the resulting detections and their measured SNRs. This is to be part of DM-6244 . This ticket is now considered Done.
            Hide
            reiss David Reiss added a comment -

            Did not check in the code.

            Show
            reiss David Reiss added a comment - Did not check in the code.
            Hide
            reiss David Reiss added a comment -

            Thanks for agreeing to do the review. Here's some info:

            It consists of a set of small-ish changes, many of which are associated with bugs that I discovered in the new imageDecorrelation code. Primarily:

            1. Ensuring that convolution kernels are correctly centered
            2. Ensuring that science and template images are correctly ordered. It turns out it didn't really matter previously as long as the variances of the two images were similar, but the confusion is all corrected now. I have also added some new unit tests to ensure this.
            3. Using variance rather than sigma**2 in all of the function calls.

            There is an additional fix in pipe_tasks that ensures that the correct matchedTemplate is being fed to the DipoleFitTask.

            Build was successful on Jenkins:
            https://ci.lsst.codes/job/stack-os-matrix/13308/

            Show
            reiss David Reiss added a comment - Thanks for agreeing to do the review. Here's some info: It consists of a set of small-ish changes, many of which are associated with bugs that I discovered in the new imageDecorrelation code. Primarily: 1. Ensuring that convolution kernels are correctly centered 2. Ensuring that science and template images are correctly ordered. It turns out it didn't really matter previously as long as the variances of the two images were similar, but the confusion is all corrected now. I have also added some new unit tests to ensure this. 3. Using variance rather than sigma**2 in all of the function calls. There is an additional fix in pipe_tasks that ensures that the correct matchedTemplate is being fed to the DipoleFitTask. Build was successful on Jenkins: https://ci.lsst.codes/job/stack-os-matrix/13308/
            Hide
            Parejkoj John Parejko added a comment - - edited

            Several comments from the review that would clean the code up. I'd definitely like to see the assertNotClose tests be made more explicit if possible.

            Show
            Parejkoj John Parejko added a comment - - edited Several comments from the review that would clean the code up. I'd definitely like to see the assertNotClose tests be made more explicit if possible.
            Hide
            Parejkoj John Parejko added a comment -

            After some discussion and updates (particularly making the tests more robust by using assertLess), I'm happy.

            Show
            Parejkoj John Parejko added a comment - After some discussion and updates (particularly making the tests more robust by using assertLess), I'm happy.
            Hide
            reiss David Reiss added a comment -

            Thanks for the thorough review, John Parejko. I have made the changes you requested and tested the changes on jenkins: https://ci.lsst.codes/job/stack-os-matrix/13404/

            Merged and done.

            Show
            reiss David Reiss added a comment - Thanks for the thorough review, John Parejko . I have made the changes you requested and tested the changes on jenkins: https://ci.lsst.codes/job/stack-os-matrix/13404/ Merged and done.

              People

              Assignee:
              reiss David Reiss
              Reporter:
              reiss David Reiss
              Reviewers:
              John Parejko
              Watchers:
              David Reiss, John Parejko
              Votes:
              0 Vote for this issue
              Watchers:
              2 Start watching this issue

                Dates

                Created:
                Updated:
                Resolved:

                  CI Builds

                  No builds found.