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

Give ModelPsfMatchTask ablilty to match to all PSF types

    Details

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

      Description

      Ticket named " 'Psf' object has no attribute 'resized'" as filed.

      ModelPsfMatchTask calls Psf.resized, but that method is only defined for certain specialised Psf classes (SingleGaussianPsf and DoubleGaussianPsf). It should have a default implementation, and be defined for CoaddPsf.

       > python modelPsfMatchTask.py --template 'data/SDSSJ0920+0034/calexp-HSC-I-9564-7,3.fits' --science 'data/SDSSJ0920+0034/calexp-HSC-R-9564-7,3.fits'
       
      psfMatch INFO: compute Psf-matching kernel
      psfMatch INFO: Adjusting dimensions of reference PSF model from (43, 43) to (41, 41)
      Traceback (most recent call last):
        File "modelPsfMatchTask.py", line 160, in <module>
          run(args)
        File "modelPsfMatchTask.py", line 130, in run
          result = psfMatchTask.run(templateExp, scienceExp)
        File "modelPsfMatchTask.py", line 45, in run
          return ModelPsfMatchTask.run(self, scienceExp, templateExp.getPsf())
        File "/Users/aisun/anaconda/envs/lsst/opt/lsst/pipe_base/python/lsst/pipe/base/timer.py", line 121, in wrapper
          res = func(self, *args, **keyArgs)
        File "/Users/aisun/anaconda/envs/lsst/opt/lsst/ip_diffim/python/lsst/ip/diffim/modelPsfMatch.py", line 274, in run
          result = self._buildCellSet(exposure, referencePsfModel)
        File "/Users/aisun/anaconda/envs/lsst/opt/lsst/ip_diffim/python/lsst/ip/diffim/modelPsfMatch.py", line 388, in _buildCellSet
          referencePsfModel = referencePsfModel.resized(lenPsfScience, lenPsfScience)
        File "/Users/aisun/anaconda/envs/lsst/opt/lsst/afw/python/lsst/afw/detection/detectionLib.py", line 3745, in <lambda>
          __getattr__ = lambda self, name: _swig_getattr(self, Psf, name)
        File "/Users/aisun/anaconda/envs/lsst/opt/lsst/afw/python/lsst/afw/detection/detectionLib.py", line 89, in _swig_getattr
          raise AttributeError("'%s' object has no attribute '%s'" % (class_type.__name__, name))
      AttributeError: 'Psf' object has no attribute 'resized'
      

      Bug reported by Ai-Lei Sun.

        Attachments

          Issue Links

            Activity

            Hide
            yusra Yusra AlSayyad added a comment -

            Duplicate of an issue we decided not to do because the assumption that ModelPsfMatchTask only needs to match to models. ImagePsfMatchTask handles the general case.

            My question is: why isn't she using imagePsfMatchTask.py?

            Show
            yusra Yusra AlSayyad added a comment - Duplicate of an issue we decided not to do because the assumption that ModelPsfMatchTask only needs to match to models. ImagePsfMatchTask handles the general case. My question is: why isn't she using imagePsfMatchTask.py?
            Hide
            price Paul Price added a comment -

            Ai Lei is attempting to PSF-match some coadds. That should be possible from the known PSF of each coadd.

            Even if you decide not to implement CoaddPsf.resized (though surely that's not difficult?), Psf.resized should be defined.

            Show
            price Paul Price added a comment - Ai Lei is attempting to PSF-match some coadds. That should be possible from the known PSF of each coadd. Even if you decide not to implement CoaddPsf.resized (though surely that's not difficult?), Psf.resized should be defined.
            Hide
            yusra Yusra AlSayyad added a comment -

            ModelPsfMatchTask has never been able to match any PSF to any PSF. Recall that four months ago it couldn't match WarpedPsfs to a simple model (which was why we matched before warping). If not the resized error, you'd get the error that the PSF sizes didn't match and/or that there were no suitable kernel candidates.

            This ticket is actually making ModelPsfMatchTask be able to match any PSF to any PSF, which as "not difficult" as you say, but also not trivial. In addition to WarpedPsf's needing to compute what size their undistortedPsf should be resized to in-order for the distorted PSF to be a requested size, PSFs are composed of Kernels and don't know until runtime what kernels they have. Kernels are responsible for the dimensions of the PSFs. To do this right requires adding some functionality to the Kernels, basic Psfs, and transformed Psfs (Warped/Coadd Psfs).

            Show
            yusra Yusra AlSayyad added a comment - ModelPsfMatchTask has never been able to match any PSF to any PSF. Recall that four months ago it couldn't match WarpedPsfs to a simple model (which was why we matched before warping). If not the resized error, you'd get the error that the PSF sizes didn't match and/or that there were no suitable kernel candidates. This ticket is actually making ModelPsfMatchTask be able to match any PSF to any PSF , which as "not difficult" as you say, but also not trivial. In addition to WarpedPsf's needing to compute what size their undistortedPsf should be resized to in-order for the distorted PSF to be a requested size, PSFs are composed of Kernels and don't know until runtime what kernels they have. Kernels are responsible for the dimensions of the PSFs. To do this right requires adding some functionality to the Kernels, basic Psfs, and transformed Psfs (Warped/Coadd Psfs).
            Hide
            yusra Yusra AlSayyad added a comment -

            Pim Schellart [X] Would you review the afw PR, because it's ready and you have some availability now?

            It adds functionality to Kernels to resize themselves. It's been a while since I've last C++'d and want to make sure I didn't do anything stupid in the method implementations. Note that this function will likely never be called in a loop.

            Show
            yusra Yusra AlSayyad added a comment - Pim Schellart [X] Would you review the afw PR, because it's ready and you have some availability now? It adds functionality to Kernels to resize themselves. It's been a while since I've last C++'d and want to make sure I didn't do anything stupid in the method implementations. Note that this function will likely never be called in a loop.
            Hide
            yusra Yusra AlSayyad added a comment - - edited

            Summary:
            4 PRs (afw, meas_algorithms, ip_diffim, meas_extensions_psfex)
            https://ci.lsst.codes/job/stack-os-matrix/25141/

            Turns out resizing to an exact kernel BBox for WarpedPsfs is impossible given the current internal representation of a WarpedPsf by an _undistortedPsf and _distortion. (See attached schematic). Rather than making a new "BBoxAwareWarpedPsf", I decided to throw Not Implemented Errors for WarpedPsf and consequently CoaddPsf which is composed of WarpedPsfs. If confronted with a WarpedPsf or CoaddPsf as a reference PSF, ModelPsfMatchTask will pad or clip the kernel image to match the science PSF. PsfExPsf was very opaque to me, so it raises Not Implemented as well.

            Pim Schellart [X] can you take the other 3 packages as well, or should I find another reviewer for those?

            Russell Owen It looks like you wrote a lot of the original afw.math.Kernel code; will you bless the interface for the resized method?
            virtual std::shared_ptr<Kernel> resized(int width, int height) const = 0;
            I could easily be convinced to use a const ref to an Extent2I if you'd prefer:
            virtual std::shared_ptr<Kernel> resized(Extent2I const& dim) const = 0;

            Show
            yusra Yusra AlSayyad added a comment - - edited Summary: 4 PRs (afw, meas_algorithms, ip_diffim, meas_extensions_psfex) https://ci.lsst.codes/job/stack-os-matrix/25141/ Turns out resizing to an exact kernel BBox for WarpedPsfs is impossible given the current internal representation of a WarpedPsf by an _undistortedPsf and _distortion . (See attached schematic). Rather than making a new "BBoxAwareWarpedPsf", I decided to throw Not Implemented Errors for WarpedPsf and consequently CoaddPsf which is composed of WarpedPsfs. If confronted with a WarpedPsf or CoaddPsf as a reference PSF, ModelPsfMatchTask will pad or clip the kernel image to match the science PSF. PsfExPsf was very opaque to me, so it raises Not Implemented as well. Pim Schellart [X] can you take the other 3 packages as well, or should I find another reviewer for those? Russell Owen It looks like you wrote a lot of the original afw.math.Kernel code; will you bless the interface for the resized method? virtual std::shared_ptr<Kernel> resized(int width, int height) const = 0; I could easily be convinced to use a const ref to an Extent2I if you'd prefer: virtual std::shared_ptr<Kernel> resized(Extent2I const& dim) const = 0;
            Hide
            pschella Pim Schellart [X] (Inactive) added a comment -

            See minor comments on PR.

            Show
            pschella Pim Schellart [X] (Inactive) added a comment - See minor comments on PR.
            Hide
            yusra Yusra AlSayyad added a comment -

            Pim Schellart [X] I implemented most of your suggestions, with the exception of the pointer modernization and override. I value consistency in each module, and so if I remove one use of the PTR macro, I'm going to do the whole module. If I do the 15 modules I touched in meas_algorithms, I might as well do all of them, and then I might as well ask Krzysztof for his script that he ran on afw for https://github.com/lsst/afw/commit/1d5e4983a7514d613d0224d80ba6220738b2a4d7 to run on meas_algorithms.

            Then pretty soon, I'm doing DM-6056 which is assigned to you.

            As you point out, for "override", we have to do either the whole module or nothing, else we get warnings.

            Show
            yusra Yusra AlSayyad added a comment - Pim Schellart [X] I implemented most of your suggestions, with the exception of the pointer modernization and override . I value consistency in each module, and so if I remove one use of the PTR macro, I'm going to do the whole module. If I do the 15 modules I touched in meas_algorithms, I might as well do all of them, and then I might as well ask Krzysztof for his script that he ran on afw for https://github.com/lsst/afw/commit/1d5e4983a7514d613d0224d80ba6220738b2a4d7 to run on meas_algorithms. Then pretty soon, I'm doing DM-6056 which is assigned to you. As you point out, for "override", we have to do either the whole module or nothing, else we get warnings.
            Hide
            yusra Yusra AlSayyad added a comment -

            I've haven't seen any trace of Russell Owen this week, and am inclined to leave the interface for the resized method as is.

            Show
            yusra Yusra AlSayyad added a comment - I've haven't seen any trace of Russell Owen this week, and am inclined to leave the interface for the resized method as is.

              People

              • Assignee:
                yusra Yusra AlSayyad
                Reporter:
                price Paul Price
                Reviewers:
                Pim Schellart [X] (Inactive)
                Watchers:
                Paul Price, Pim Schellart [X] (Inactive), Yusra AlSayyad
              • Votes:
                0 Vote for this issue
                Watchers:
                3 Start watching this issue

                Dates

                • Created:
                  Updated:
                  Resolved:

                  Summary Panel