Skip to content
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

DM-40668: Update computation of psfStarScaledDeltaSizeScatter #843

Merged
merged 6 commits into from
Nov 8, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions python/lsst/pipe/tasks/calibrateImage.py
Original file line number Diff line number Diff line change
Expand Up @@ -319,7 +319,9 @@ def setDefaults(self):
self.photometry.match.sourceSelection.retarget(sourceSelector.NullSourceSelectorTask)

# All sources should be good for PSF summary statistics.
# TODO: These should both be changed to calib_psf_used with DM-41640.
self.compute_summary_stats.starSelection = "calib_photometry_used"
self.compute_summary_stats.starSelector.flags.good = ["calib_photometry_used"]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, I see, thanks!



class CalibrateImageTask(pipeBase.PipelineTask):
Expand Down
55 changes: 46 additions & 9 deletions python/lsst/pipe/tasks/computeExposureSummaryStats.py
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@
import lsst.afw.math as afwMath
import lsst.afw.image as afwImage
import lsst.geom as geom
from lsst.meas.algorithms import ScienceSourceSelectorTask
from lsst.utils.timer import timeMethod


Expand All @@ -55,9 +56,13 @@ class ComputeExposureSummaryStatsConfig(pexConfig.Config):
default=("NO_DATA", "SUSPECT"),
)
starSelection = pexConfig.Field(
doc="Field to select sources to be used in the PSF statistics computation.",
doc="Field to select full list of sources used for PSF modeling.",
dtype=str,
default="calib_psf_used"
default="calib_psf_used",
)
starSelector = pexConfig.ConfigurableField(
target=ScienceSourceSelectorTask,
doc="Selection of sources to compute PSF star statistics.",
)
starShape = pexConfig.Field(
doc="Base name of columns to use for the source shape in the PSF statistics computation.",
Expand Down Expand Up @@ -88,6 +93,27 @@ class ComputeExposureSummaryStatsConfig(pexConfig.Config):
default=("BAD", "CR", "EDGE", "INTRP", "NO_DATA", "SAT", "SUSPECT"),
)

def setDefaults(self):
super().setDefaults()

self.starSelector.setDefaults()
self.starSelector.doFlags = True
self.starSelector.doSignalToNoise = True
self.starSelector.doUnresolved = False
self.starSelector.doIsolated = False
self.starSelector.doRequireFiniteRaDec = False
self.starSelector.doRequirePrimary = False

self.starSelector.signalToNoise.minimum = 50.0
self.starSelector.signalToNoise.maximum = 1000.0

self.starSelector.flags.bad = ["slot_Shape_flag", "slot_PsfFlux_flag"]
# Select stars used for PSF modeling.
self.starSelector.flags.good = ["calib_psf_used"]

self.starSelector.signalToNoise.fluxField = "slot_PsfFlux_instFlux"
self.starSelector.signalToNoise.errField = "slot_PsfFlux_instFluxErr"


class ComputeExposureSummaryStatsTask(pipeBase.Task):
"""Task to compute exposure summary statistics.
Expand Down Expand Up @@ -129,6 +155,11 @@ class ComputeExposureSummaryStatsTask(pipeBase.Task):
ConfigClass = ComputeExposureSummaryStatsConfig
_DefaultName = "computeExposureSummaryStats"

def __init__(self, **kwargs):
super().__init__(**kwargs)

self.makeSubtask("starSelector")

@timeMethod
def run(self, exposure, sources, background):
"""Measure exposure statistics from the exposure, sources, and
Expand Down Expand Up @@ -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 total number of psf stars used (prior to stats selection).
nPsfStar = sources[self.config.starSelection].sum()
summary.nPsfStar = int(nPsfStar)

psf_mask = self.starSelector.run(sources).selected
nPsfStarsUsedInStats = psf_mask.sum()

if nPsfStar == 0:
if nPsfStarsUsedInStats == 0:
# No stars to measure statistics, so we must return the defaults
# of 0 stars and NaN values.
return
Expand All @@ -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.)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, yeah...agreed!


starE1 = (starXX - starYY)/(starXX + starYY)
starE2 = 2*starXY/(starXX + starYY)
starSizeMedian = np.median(starSize)

psfSize = (psfXX*psfYY - psfXY**2)**0.25
# Use the trace radius for the psf size.
psfSize = np.sqrt(psfXX/2. + psfYY/2.)
psfE1 = (psfXX - psfYY)/(psfXX + psfYY)
psfE2 = 2*psfXY/(psfXX + psfYY)

Expand All @@ -279,9 +317,8 @@ def update_psf_stats(self, summary, psf, bbox, sources=None, image_mask=None, so

psfStarDeltaSizeMedian = np.median(starSize - psfSize)
psfStarDeltaSizeScatter = sigmaMad(starSize - psfSize, scale='normal')
psfStarScaledDeltaSizeScatter = psfStarDeltaSizeScatter/starSizeMedian**2.
psfStarScaledDeltaSizeScatter = psfStarDeltaSizeScatter/starSizeMedian

summary.nPsfStar = int(nPsfStar)
summary.psfStarDeltaE1Median = float(psfStarDeltaE1Median)
summary.psfStarDeltaE2Median = float(psfStarDeltaE2Median)
summary.psfStarDeltaE1Scatter = float(psfStarDeltaE1Scatter)
Expand Down
2 changes: 1 addition & 1 deletion python/lsst/pipe/tasks/selectImages.py
Original file line number Diff line number Diff line change
Expand Up @@ -258,7 +258,7 @@ class PsfWcsSelectImagesConfig(pipeBase.PipelineTaskConfig,
maxScaledSizeScatter = pexConfig.Field(
doc="Maximum scatter in the size residuals, scaled by the median size",
dtype=float,
default=0.009,
default=0.019,
optional=True,
)
maxPsfTraceRadiusDelta = pexConfig.Field(
Expand Down