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

Fix occasional 1-pixel shifts in Zogy

    XMLWordPrintable

    Details

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

      Description

      Occasionally seen with both Zogy and AL Decorrelation: depending upon the dimensions of PSFs/kernels/input images, the resulting Zogy diffim or decorrelated diffim can have an erroneous 1-pixel offset (shift). Several attempts have been made to fix this but it still occurs on occasion.

        Attachments

          Activity

          Hide
          wmwood-vasey Michael Wood-Vasey [X] (Inactive) added a comment - - edited

          My student, Shu Liu, has a fix for this for the ZOGY algorithm.

          Fix the one pixel offset bug.
          1. When padding a PSF with (odd, odd) dimension to (even, even) dimension, the center of the PSF should be placed to the center-right of the array. Otherwise the peak value could be shifted to the end of the array after applying np.fft.fftshift(e.g. Correct padding: [0,0,1,2,3,2,1,0] -> [3,2,1,0,0,0,1,2], Incorrect padding: [0,1,2,3,2,1,0,0] - > [2,1,0,0, 0,1,2,3]). This corresponds to a one pixel offset since the np.fft.fft assume the first component of the array as the origin.
          2. Add a test file for checking the pixel offset of the ZOGY D image.

          Show
          wmwood-vasey Michael Wood-Vasey [X] (Inactive) added a comment - - edited My student, Shu Liu, has a fix for this for the ZOGY algorithm. Fix the one pixel offset bug. 1. When padding a PSF with (odd, odd) dimension to (even, even) dimension, the center of the PSF should be placed to the center-right of the array. Otherwise the peak value could be shifted to the end of the array after applying np.fft.fftshift(e.g. Correct padding: [0,0,1,2,3,2,1,0] -> [3,2,1,0,0,0,1,2] , Incorrect padding: [0,1,2,3,2,1,0,0] - > [2,1,0,0, 0,1,2,3] ). This corresponds to a one pixel offset since the np.fft.fft assume the first component of the array as the origin. 2. Add a test file for checking the pixel offset of the ZOGY D image.
          Hide
          wmwood-vasey Michael Wood-Vasey [X] (Inactive) added a comment -

          Gabor Kovacs [X] Am I correct in recalling that you put in a fix for this for A&L?

          Show
          wmwood-vasey Michael Wood-Vasey [X] (Inactive) added a comment - Gabor Kovacs [X] Am I correct in recalling that you put in a fix for this for A&L?
          Show
          wmwood-vasey Michael Wood-Vasey [X] (Inactive) added a comment - tickets/ DM-11990 branch passes Jenkins: https://ci.lsst.codes/blue/organizations/jenkins/stack-os-matrix/detail/stack-os-matrix/31311/pipeline
          Hide
          swinbank John Swinbank added a comment -

          Thanks Shu Liu (and Michael)!

          Show
          swinbank John Swinbank added a comment - Thanks Shu Liu (and Michael)!
          Hide
          gkovacs Gabor Kovacs [X] (Inactive) added a comment - - edited

          I agree with your "odd psf" to even size" padding instructions. Indeed, I also arrived at the conclusion that the center PSF pixel should be at (0,0) after fftshift.

          I'm happy with the merging. Please rebase and squash the commits into one commit with a descriptive message (please find details in the the developer guide here about this).

           

          Show
          gkovacs Gabor Kovacs [X] (Inactive) added a comment - - edited I agree with your "odd psf" to even size" padding instructions. Indeed, I also arrived at the conclusion that the center PSF pixel should be at (0,0) after fftshift. I'm happy with the merging. Please rebase and squash the commits into one commit with a descriptive message (please find details in the the developer guide here about this).  
          Hide
          gkovacs Gabor Kovacs [X] (Inactive) added a comment - - edited

          Gabor Kovacs [X] Am I correct in recalling that you put in a fix for this for A&L?

          Yes, the decorrelation afterburner is being changed (DM-21868), including this issue. I renamed this ticket to refer to zogy only.

          Show
          gkovacs Gabor Kovacs [X] (Inactive) added a comment - - edited Gabor Kovacs [X] Am I correct in recalling that you put in a fix for this for A&L? Yes, the decorrelation afterburner is being changed ( DM-21868 ), including this issue. I renamed this ticket to refer to zogy only.
          Hide
          wmwood-vasey Michael Wood-Vasey [X] (Inactive) added a comment -

          Merged to master.

          Gabor Kovacs [X] Thanks for the review and thanks for updating the name of this issue to be more specific.

          Show
          wmwood-vasey Michael Wood-Vasey [X] (Inactive) added a comment - Merged to master. Gabor Kovacs [X] Thanks for the review and thanks for updating the name of this issue to be more specific.

            People

            Assignee:
            wmwood-vasey Michael Wood-Vasey [X] (Inactive)
            Reporter:
            reiss David Reiss
            Reviewers:
            Gabor Kovacs [X] (Inactive)
            Watchers:
            David Reiss, Gabor Kovacs [X] (Inactive), John Swinbank, Michael Wood-Vasey [X] (Inactive)
            Votes:
            0 Vote for this issue
            Watchers:
            4 Start watching this issue

              Dates

              Created:
              Updated:
              Resolved:

                CI Builds

                No builds found.