# Implement GAaP: Gaussian Aperture and PSF photometry

XMLWordPrintable

#### Details

• Type: Story
• Status: Done
• Resolution: Done
• Fix Version/s: None
• Component/s: None
• Labels:
• Story Points:
32
• Team:
Data Release Production
• Urgent?:
No

#### Description

In addition to Kron and CModel photometry, this ticket proposes to implement GAaP photometry (Kuijken 2008). For extended objects (galaxies), GAaP is not designed to give accurate fluxes, but provides accurate and robust colors. This ticket is a master-ticket to contain multiple tickets related to this work.

#### Activity

Hide
Jim Bosch added a comment -

I'm done reviewing, and while I've left a ton of comments on the PR, overall I think this looks pretty good, and the volume of comments just reflects this being your first big PR and the fact that it's in a sort of crusty part of the codebase (measurement plugins), where our best examples are sort of fossilized relative to modern style.

I've looked through all of this pretty throughly, so one approach to splitting up with the review with Yusra AlSayyad might be for her to take the second pass, after you think you've addressed my comments?

Show
Jim Bosch added a comment - I'm done reviewing, and while I've left a ton of comments on the PR, overall I think this looks pretty good, and the volume of comments just reflects this being your first big PR and the fact that it's in a sort of crusty part of the codebase (measurement plugins), where our best examples are sort of fossilized relative to modern style. I've looked through all of this pretty throughly, so one approach to splitting up with the review with Yusra AlSayyad might be for her to take the second pass, after you think you've addressed my comments?
Hide

That sounds good to me. I probably won't be pushing any new changes to the Github repo until Monday, so if Yusra likes to have a first pass, she can do that before Monday and after that, it's probably best waiting for me to address Jim's comments.

Show
Arun Kannawadi added a comment - That sounds good to me. I probably won't be pushing any new changes to the Github repo until Monday, so if Yusra likes to have a first pass, she can do that before Monday and after that, it's probably best waiting for me to address Jim's comments.
Hide

Along with DM-28955, I have modified the plugin so that the modelPsfMatch task is constructed once during the plugin initialization and the alardSigGauss parameter can be called during the run time. I am not creating the PR on DM-28955 at the moment, because the size parameter of the AL Basis can be chosen dynamically using the scaleByFwhm parameter (bad naming that kept it hidden to me all this while). However, it is a different choice from one that Konraad chose to implement in his GAaP code. I'm replicating his choices as closely as possible, but I'd like to investigate how well the PSF Gaussianization is if I simply set scaleByFwhm=True.

Show
Arun Kannawadi added a comment - Along with DM-28955 , I have modified the plugin so that the modelPsfMatch task is constructed once during the plugin initialization and the alardSigGauss parameter can be called during the run time. I am not creating the PR on DM-28955 at the moment, because the size parameter of the AL Basis can be chosen dynamically using the scaleByFwhm parameter (bad naming that kept it hidden to me all this while). However, it is a different choice from one that Konraad chose to implement in his GAaP code. I'm replicating his choices as closely as possible, but I'd like to investigate how well the PSF Gaussianization is if I simply set scaleByFwhm=True.
Hide

Posted some comments on github and removed myself from the reviewer list. Main complaint is that it doesn't build.

Show
Yusra AlSayyad added a comment - Posted some comments on github and removed myself from the reviewer list. Main complaint is that it doesn't build.
Hide

I've fixed the build issue and made sure it builds successfully in a cloned repo as well. I made one other commit during pair coding with Jim this Wednesday. I think I've also addressed all other minor comments. I'll merge it to master on Monday and leave any remaining comments for smaller tickets later. This is going to remain an lsst-dm repo for some more time, at least until I wrap up DM-28955.

Show
Arun Kannawadi added a comment - I've fixed the build issue and made sure it builds successfully in a cloned repo as well. I made one other commit during pair coding with Jim this Wednesday. I think I've also addressed all other minor comments. I'll merge it to master on Monday and leave any remaining comments for smaller tickets later. This is going to remain an lsst-dm repo for some more time, at least until I wrap up DM-28955 .

#### People

Assignee:
Reporter:
Reviewers:
Jim Bosch
Watchers: