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

Fix oversampling settings in psfex

    XMLWordPrintable

    Details

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

      Description

      The current settings in psfex will only turn on oversampling only if the seeing is < 0.5", even if you have configured it do oversampling. This needs to be changed so that everything is determined by the config parameters.

      We have also seen on HSC data that oversampling in general does not work well in psfex. We need to change the current configuration which does 2x oversampling to just use the native pixel scale.

        Attachments

          Issue Links

            Activity

            Hide
            price Paul Price added a comment -

            The unit test also fails on the HSC side (but unit test failures there aren't fatal to the build, so it seems no-one noticed).

            Show
            price Paul Price added a comment - The unit test also fails on the HSC side (but unit test failures there aren't fatal to the build, so it seems no-one noticed).
            Hide
            swinbank John Swinbank added a comment - - edited

            Assuming that the test fails in an expected way – ie, you and Bob Armstrong are happy that you can explain the failure and can modify the test case appropriately – I have no problem with this being merged. If you need further changes to the code, rather than just adjust the numbers in the test, by all means send it back to me for another look.

            Show
            swinbank John Swinbank added a comment - - edited Assuming that the test fails in an expected way – ie, you and Bob Armstrong are happy that you can explain the failure and can modify the test case appropriately – I have no problem with this being merged. If you need further changes to the code, rather than just adjust the numbers in the test, by all means send it back to me for another look.
            Hide
            price Paul Price added a comment -

            I found the cause of the test failure — oversampling hadn't been disabled in the test.

            pprice@tiger-sumire:~/LSST/meas/extensions/psfex (tickets/DM-6982=) $ git --no-pager log -2 --reverse
            commit a8046b3778507c70327a79b4fc2d48a30ad7cfc8
            Author: Paul Price <price@astro.princeton.edu>
            Date:   Tue Jul 26 16:41:00 2016 -0400
             
                disable oversampling in test
                
                We believe that oversampling doesn't work well. Indeed, with
                oversampling turned off, we get better fits for the test image.
             
            commit 164e2651a573d7314ca865f41d46b8f68f7cbcfb
            Author: Paul Price <price@astro.princeton.edu>
            Date:   Tue Jul 26 16:41:14 2016 -0400
             
                remove unused and confusing line in test
                
                self.FWHM is unused, and its presence is confusing because there
                is also sigma1 and sigma2 which are used to set the PSF, and they
                have no relation to self.FWHM.
            

            Show
            price Paul Price added a comment - I found the cause of the test failure — oversampling hadn't been disabled in the test. pprice@tiger-sumire:~/LSST/meas/extensions/psfex (tickets/DM-6982=) $ git --no-pager log -2 --reverse commit a8046b3778507c70327a79b4fc2d48a30ad7cfc8 Author: Paul Price <price@astro.princeton.edu> Date: Tue Jul 26 16:41:00 2016 -0400   disable oversampling in test We believe that oversampling doesn't work well. Indeed, with oversampling turned off, we get better fits for the test image.   commit 164e2651a573d7314ca865f41d46b8f68f7cbcfb Author: Paul Price <price@astro.princeton.edu> Date: Tue Jul 26 16:41:14 2016 -0400   remove unused and confusing line in test self.FWHM is unused, and its presence is confusing because there is also sigma1 and sigma2 which are used to set the PSF, and they have no relation to self.FWHM.
            Hide
            swinbank John Swinbank added a comment -

            Line 231 of testPsfexPsf.py is a comment which reads "Include oversampling in test". It immediately precedes the lines you've edited in a8046b3 to "disable oversampling in test". Please make these consistent!

            I note that the test passes if you simply use the default kernelSize and samplingSize rather than overriding them in the test case. Is the override actually necessary? Can you just remove it (and the comment) for simplicity?

            Show
            swinbank John Swinbank added a comment - Line 231 of testPsfexPsf.py is a comment which reads "Include oversampling in test". It immediately precedes the lines you've edited in a8046b3 to "disable oversampling in test". Please make these consistent! I note that the test passes if you simply use the default kernelSize and samplingSize rather than overriding them in the test case. Is the override actually necessary? Can you just remove it (and the comment) for simplicity?
            Hide
            price Paul Price added a comment -

            I'm such a doofus. I remember thinking that I could just remove those lines, but must have gotten side-tracked. They're gone now, the test passes, and I've merged to master.

            Show
            price Paul Price added a comment - I'm such a doofus. I remember thinking that I could just remove those lines, but must have gotten side-tracked. They're gone now, the test passes, and I've merged to master.

              People

              Assignee:
              price Paul Price
              Reporter:
              rearmstr Bob Armstrong
              Reviewers:
              John Swinbank
              Watchers:
              Bob Armstrong, John Swinbank, Paul Price
              Votes:
              0 Vote for this issue
              Watchers:
              3 Start watching this issue

                Dates

                Created:
                Updated:
                Resolved:

                  CI Builds

                  No builds found.