DM-40668: Update computation of psfStarScaledDeltaSizeScatter #843
Conversation
erykoff
force-pushed
the
tickets/DM-40668
branch
2 times, most recently
from
f49c5e7 to
023cbec
Compare
| @@ -320,6 +320,7 @@ def setDefaults(self): | |||
|
|
|||
| # All sources should be good for PSF summary statistics. | |||
| self.compute_summary_stats.starSelection = "calib_photometry_used" | |||
| self.compute_summary_stats.starSelector.flags.good = ["calib_photometry_used"] | |||
There was a problem hiding this comment.
This feels redundant...and why photometry instead of psf here? The docstring explicitly reads:
starSelection = pexConfig.Field(
doc="Field to select full list of sources used for PSF modeling.",
now (i.e with the change you made in this ticket).
There was a problem hiding this comment.
This is actually a bug in calibrateImage that I discussed with John P. yesterday that it doesn't propagate the psf used flag. This will be fixed when that is fixed. I'll add a to-do
| @@ -263,12 +298,15 @@ def update_psf_stats(self, summary, psf, bbox, sources=None, image_mask=None, so | |||
| psfYY = psf_cat[self.config.psfShape + '_yy'] | |||
| psfXY = psf_cat[self.config.psfShape + '_xy'] | |||
|
|
|||
| starSize = (starXX*starYY - starXY**2.)**0.25 | |||
| # Use the trace radius for the star size. | |||
| starSize = np.sqrt(starXX/2. + starYY/2.) | |||
There was a problem hiding this comment.
Given the ambiguity of Size, it may be worth it to explicitly name the variable with TraceRadius (those reading code below might miss your comment here and assume their favorite meaning of starSize)?
There was a problem hiding this comment.
I can change the internal variables to make the code cleaner. But I don't want to rename all the fields unless I really have to.
| @@ -243,10 +274,14 @@ def update_psf_stats(self, summary, psf, bbox, sources=None, image_mask=None, so | |||
| # good sources). | |||
| return | |||
|
|
|||
| psf_mask = sources[self.config.starSelection] & (~sources[self.config.starShape + '_flag']) | |||
| nPsfStar = psf_mask.sum() | |||
| # Count the number of raw psf stars | |||
There was a problem hiding this comment.
The meaning of "raw" is not clear to me here.
|
|
||
| if nPsfStar == 0: | ||
| if nPsfStarCut == 0: |
There was a problem hiding this comment.
Should this be set a bit higher? (i.e. do you feel that having only 1 or 2 stars is really "enough" for anything meaningful?)
There was a problem hiding this comment.
And maybe something more explicit, e.g. nPsfStarsUsedInStats?
There was a problem hiding this comment.
Interesting question; I am inclined to use it, the values can be computed ...
There was a problem hiding this comment.
Ok...I guess I'm also a bit uncomfortable that we don't record how many stars were used to compute the metrics (and the danger of thinking nPsfStars is implying that, which it's not). But this isn't your issue to deal with here!
This default cut of 0.019 has been determined from the reprocessing of HSC PDR2.
erykoff
force-pushed
the
tickets/DM-40668
branch
from
023cbec to
9214a04
Compare
This PR updates a few things for
psfStarScaledDeltaSizeScatter:a) Object sizes are now computed with the trace radius rather than the determinant radius (less noisy)
b) An explicit selector can be used to control the signal to noise of the size statistics.
c) The scaling factor is changed from size squared to size.
d) The default cut applied for coadds has been updated to 0.019 appropriate for this new definition, as determined from the reprocessing of HSC PDR2.