# Give ModelPsfMatchTask ablilty to match to all PSF types

XMLWordPrintable

## Details

• Type: Story
• Status: Done
• Resolution: Done
• Fix Version/s: None
• Component/s:
• Labels:
None
• Story Points:
8
• Sprint:
DRP F17-2
• Team:
Data Release Production

## 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   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   __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.

## Activity

Hide

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 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
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
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

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 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

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 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

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 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
Pim Schellart [X] (Inactive) added a comment -

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

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 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

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 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:
Reporter:
Paul Price
Reviewers:
Pim Schellart [X] (Inactive)
Watchers:
Paul Price, Pim Schellart [X] (Inactive), Yusra AlSayyad