DM-32649: Add psf size and shape residual statistics to visitSummary #604
Conversation
| doc="Name of psf shape", | ||
| dtype=str, | ||
| default='base_SdssShape_psf' | ||
| ) |
There was a problem hiding this comment.
Pet peeve: why mix between single and double quotes for strings (I generally prefer the latter).
There was a problem hiding this comment.
...because I literally cut-and-pasted from the prior art which had the mix. And while I have a lot of code style opinions, strangely single-or-double spaces is not something I notice. Anyway, I'll fix to double quotes.
| @@ -53,6 +54,21 @@ class ComputeExposureSummaryStatsConfig(pexConfig.Config): | |||
| doc="Mask planes that, if set, the associated pixel should not be included sky noise calculation.", | |||
| default=("NO_DATA", "SUSPECT"), | |||
| ) | |||
| starSelection = pexConfig.Field( | |||
| doc="Field to select stars.", | |||
There was a problem hiding this comment.
Could be a tad more descriptive, e.g.:
"Field to select sources to be used in the PSF statistics computation."?
| default='calib_psf_used' | ||
| ) | ||
| starShape = pexConfig.Field( | ||
| doc="Name of star shape", |
There was a problem hiding this comment.
Could be a tad more descriptive, e.g.:
"Name of column to use for the source shape in the PSF statistics computation"?
| default='base_SdssShape' | ||
| ) | ||
| psfShape = pexConfig.Field( | ||
| doc="Name of psf shape", |
There was a problem hiding this comment.
Could be a tad more descriptive, e.g.:
"Name of column to use for the PSF shape in the PSF statistics computation"?
| @@ -191,6 +207,9 @@ def run(self, exposure, sources, background): | |||
| astromOffsetMean = np.nan | |||
| astromOffsetStd = np.nan | |||
|
|
|||
| psfStats = self._computePsfStats(sources) | |||
| scaledSize = psfStats.psfStarScaledDeltaSizeScatter | |||
There was a problem hiding this comment.
Is this just because of line-length issues? It looks like a snowflake ❄️ ! Would flake8 (and you!) be OK with carriage returning after the opening bracket?, I.e.
summary = afwImage.ExposureSummaryStats(
psfSigma=float(psfSigma),
psfArea=float(psfArea),
psfIxx=float(psfIxx),
psfIyy=float(psfIyy),
psfIxy=float(psfIxy),
ra=float(ra),
decl=float(decl),
zenithDistance=float(zenithDistance),
zeroPoint=float(zeroPoint),
skyBg=float(skyBg),
skyNoise=float(skyNoise),
meanVar=float(meanVar),
raCorners=raCorners,
decCorners=decCorners,
astromOffsetMean=astromOffsetMean,
astromOffsetStd=astromOffsetStd,
nPsfStar=psfStats.nPsfStar,
psfStarDeltaE1Median=psfStats.psfStarDeltaE1Median,
psfStarDeltaE2Median=psfStats.psfStarDeltaE2Median,
psfStarDeltaE1Scatter=psfStats.psfStarDeltaE1Scatter,
psfStarDeltaE2Scatter=psfStats.psfStarDeltaE2Scatter,
psfStarDeltaSizeMedian=psfStats.psfStarDeltaSizeMedian,
psfStarDeltaSizeScatter=psfStats.psfStarDeltaSizeScatter,
psfStarScaledDeltaSizeScatter=psfStats.psfStarScaledDeltaSizeScatter,
)
| psfStats.psfStarDeltaE2Scatter = float(psfStarDeltaE2Scatter) | ||
| psfStats.psfStarDeltaSizeMedian = float(psfStarDeltaSizeMedian) | ||
| psfStats.psfStarDeltaSizeScatter = float(psfStarDeltaSizeScatter) | ||
| psfStats.psfStarScaledDeltaSizeScatter = float(psfStarScaledDeltaSizeScatter) |
There was a problem hiding this comment.
Just out of curiosity, why is all this casting needed?
There was a problem hiding this comment.
I've added a comment that we must cast to python quantities rather than numpy so that these can get serialized as yaml.
| if config.select.target != lsst.pipe.tasks.selectImages.PsfWcsSelectImagesTask: | ||
| self.inputs.remove("srcList") | ||
| self.inputs.remove("visitSummary") |
There was a problem hiding this comment.
So the comment that you removed implies to me that this connection should actually be added to the PsfWcsSelectImagesTask itself?
There was a problem hiding this comment.
You're right, I should not have removed that to-do.
| self.log.info("Removing visit %d detector %d because scaled size scatter too large: %f vs %f", | ||
| row['visit'], detectorId, scaledScatterSize, self.config.maxScaledSizeScatter) | ||
| valid = False | ||
|
|
There was a problem hiding this comment.
Again with the single & double quote mixing for strings (throughout)!
| """Should this ccd be selected based on its PSF shape information. | ||
|
|
||
| This routine is only used in Gen2 processing, and can be | ||
| removed when Gen2 is retired. |
| psfStarDeltaE2Scatter=psfStats.psfStarDeltaE2Scatter, | ||
| psfStarDeltaSizeMedian=psfStats.psfStarDeltaSizeMedian, | ||
| psfStarDeltaSizeScatter=psfStats.psfStarDeltaSizeScatter, | ||
| psfStarScaledDeltaSizeScatter=scaledSize) |
There was a problem hiding this comment.
Add the new entries to the class description?
Also, maybe that line should now read "...at the detector centers or averaged over a detector..."?
erykoff
force-pushed
the
tickets/DM-32649
branch
from
0f36b68
to
9ac6cc2
Compare
erykoff
force-pushed
the
tickets/DM-32649
branch
from
5c3a69b
to
3e9ef1a
Compare
This PR moves the gen3
PsfWcsSelectImagesTaskto using the visitSummary table and removes the source connection, which should speed up i/o.For gen2, we don't have the visitSummary tables so I left in the legacy computation, and this can be removed with the rest of the gen2 code when the time comes.