New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Set the PSF-matched coadd's PSF to the model PSF used #110
Conversation
@@ -619,7 +619,11 @@ def assembleMetadata(self, coaddExposure, tempExpRefList, weightList): | |||
self.inputRecorder.addVisitToCoadd(coaddInputs, tempExp, weight) | |||
coaddInputs.visits.sort() | |||
if self.config.doPsfMatch: | |||
psf = self.config.modelPsf.apply() | |||
# set PSF to modelPsf with maximum dimensions |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In addition to stating what the code is doing here, could you add a brief comment as to the "why"?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How much do you need here? Would it be sufficient to add explanation on why maximum dimensions in the commit message or would an inline message like the following help?
# modelPsf's BBox was dynamically defined by ModelPsfMatchTask
# to be the square bounding the largest Warped Psf BBox
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Both would be great! If the commit message is descriptive enough, the inline can be kept as brief as possible to provide the basic reasoning.
@@ -179,16 +179,17 @@ def createTempExp(self, calexpRefList, skyInfo, visitId=0): | |||
if numGoodPix > 0 and not didSetMetadata: | |||
coaddTempExp.setCalib(exposure.getCalib()) | |||
coaddTempExp.setFilter(exposure.getFilter()) | |||
# PSF replaced after loop with CoaddPsf if and only if creating direct warp |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To avoid confusion (which I was subject to at first!), perhaps rephrase as "PSF replaced with CoaddPsf after loop..."
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
# set PSF to modelPsf with maximum dimensions | ||
modelPsfList = [tempExp.getPsf() for tempExp in tempExpList] | ||
modelPsfDimList = [modelPsf.computeBBox().getDimensions() for modelPsf in modelPsfList] | ||
maxDim = max(modelPsfDimList) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm surprised max(dimList)
works. I can't figure out why it does, but I'm not familiar with pybind and maybe I missed something.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changed to be clearer and more explicit
MakeCoaddTempExp does not require users to know the correct dimensions of the model PSF and automatically adjusts the dimensions to match those of the pre-matched Warped PSFs. AssembleCoadd now applies the dimension-adjusted PSF model which was actually used rather than the user-supplied PSF model.
f6b2b3f
to
cb0fdd4
Compare
MakeCoaddTempExp does not require users to know the dimensions of
the model PSF and automatically adjusts the dimensions to match those of
the Warped PSFs. AssembleCoadd now applies the
dimension-adjusted PSF rather than the user-supplied.