Details
-
Type:
Story
-
Status: Done
-
Resolution: Done
-
Fix Version/s: None
-
Component/s: meas_algorithms
-
Labels:None
-
Story Points:8
-
Epic Link:
-
Sprint:DRP S19-1, DRP S19-2
-
Team:Data Release Production
Description
Currently, the objectSizeStarSelector Task allows for source selection based on flux limits. A selection on signal-to-noise (S/N), i.e. flux/fluxErr, is often preferred, so should be added as an option for this source selector.
Additionally, there is currently no unittest for objectSizeStarSelector, so a minimal one should be added (following the test_astrometrySourceSelector.py and test_matcherSourceSelector.py examples).
For this ticket, the option will simply be added in the same manner as the flux selection is done, but all current defaults will be left as is. Looking towards the future, I anticipate submitting an RFC to make S/N selection the default (instead of a flux-based selection, but, in principle, both could be activated simultaneously) as well as adapting all the SourceSelectorTask to make use of the configuration-based selection classes in BaseSourceSelector-derived Tasks (e.g. objectSizeStarSelectorTask, AstrometrySourceSelectorTask, MatcherSourceSelectorTask, FlaggedSourceSelectorConfig) https://github.com/lsst/meas_algorithms/blob/master/python/lsst/meas/algorithms/sourceSelector.py#L153).
Attachments
Issue Links
- relates to
-
- Done
Activity
| Field | Original Value | New Value |
|---|---|---|
| Epic Link |
|
| Status | To Do [ 10001 ] | In Progress [ 3 ] |
| Description |
Currently, the *objectSizeStarSelector* Task allows for source selection based on flux limits. A selection on signal-to-noise (S/N), i.e. flux/fluxErr, is often preferred, so should be added as an option for this source selector.
Additionally, there is currently no unittest for *objectSizeStarSelector*, so a minimal one should be added (following the *test_astrometrySourceSelector.py* and *test_matcherSourceSelector.py* examples). For this ticket, the option will simply be added in the same manner as the flux selection is done, but all current defaults will be left as is. Looking towards the future, I anticipate submitting an RFC to make S/N selection the default (instead of a flux-based selection, but, in principle, both could be activated simultaneously) as well as adapting all the *StarSelectorTask* to make use of the configuration-based selection classes in *BaseSourceSelector*-derived Tasks (e.g. *objectSizeStarSelectorTask*, *AstrometrySourceSelectorTask*, *MatcherSourceSelectorTask*, *FlaggedSourceSelectorConfig*) [https://github.com/lsst/meas_algorithms/blob/master/python/lsst/meas/algorithms/sourceSelector.py#L153]). |
Currently, the *objectSizeStarSelector* Task allows for source selection based on flux limits. A selection on signal-to-noise (S/N), i.e. flux/fluxErr, is often preferred, so should be added as an option for this source selector.
Additionally, there is currently no unittest for *objectSizeStarSelector*, so a minimal one should be added (following the *test_astrometrySourceSelector.py* and *test_matcherSourceSelector.py* examples). For this ticket, the option will simply be added in the same manner as the flux selection is done, but all current defaults will be left as is. Looking towards the future, I anticipate submitting an RFC to make S/N selection the default (instead of a flux-based selection, but, in principle, both could be activated simultaneously) as well as adapting all the *SourceSelectorTask* to make use of the configuration-based selection classes in *BaseSourceSelector*-derived Tasks (e.g. *objectSizeStarSelectorTask*, *AstrometrySourceSelectorTask*, *MatcherSourceSelectorTask*, *FlaggedSourceSelectorConfig*) [https://github.com/lsst/meas_algorithms/blob/master/python/lsst/meas/algorithms/sourceSelector.py#L153]). |
Of course you can! I also need to add a check on the source "width" (which I currently do by making it the generic BaseLimit class). The change to make use of the source selectors (and the tasks already defined in sourceSelector.py) will mess up some config parameters/overrides, so I'm not going to do the adaptation until I've got RFC approval (i.e. not in this ticket). I do suspect that review will come your way (
), so you can let me know then if all ends up in line with what you had envisioned.
| Attachment | plot-v1228-base_PsfFlux-hist.png [ 36546 ] |
| Attachment | plot-v1228-base_PsfFluxSn-hist.png [ 36547 ] |
To justify/explain my choice of signalToNoiseMin=20 for the default, I took the HSC fluxMin=4000 override (default there is 12500 which seems WAY too high! See also discussion on DM-16785) and looked for the closest equivalent in S/N. There are two grey dashed vertical lines at flux = 4000 and 12500 in the following histogram of raw Flux:
The hatched histograms have had S/N cuts as indicated in the legend. Clearly, the S/N>20 (pink-ish hatched histogram) is a good match.
This will be for a future RFC, but while I'm at it, to justify preferring a S/N cut over a flux cut, the following histogram of S/N = rawFlux/rawFluxErr shows that the current fluxMin=4000 for HSC allows some very low S/N candidates into the selection (pink-ish hatched histogram):
Would you mind giving this a look? The PR is here and Jenkins is happy.
| Reviewers | Chris Morrison [ cmorrison ] | |
| Status | In Progress [ 3 ] | In Review [ 10004 ] |
See github for review.
| Status | In Review [ 10004 ] | Reviewed [ 10101 ] |
| Sprint | DRP S19-1 [ 822 ] | DRP S19-1, DRP S19-2 [ 822, 840 ] |
Thanks again for the review. Jenkins passed. Merged to master
| Resolution | Done [ 10000 ] | |
| Status | Reviewed [ 10101 ] | Done [ 10002 ] |
| Story Points | 3 | 8 |
| Link | This issue relates to PREOPS-885 [ PREOPS-885 ] |
| Attachment | plot-v1228-base_PsfFluxSn_raw-hist.png [ 54925 ] |
| Attachment | plot-v1228-base_PsfFlux_raw-hist.png [ 54926 ] |
In DM-32624, it was noted that the fluxes plotted in the (now old) versions of the plots attached to this ticket are actually not representative of what went into the PSF modeling. They show the PsfFlux values from the src catalogs, but, of course, these are not the same as what went into the source modeling (as they aren't available without the PSF model!). Rather, what should be plotted is the PsfFlux values from the icSrc catalogs (where the "PSF" is some best guess place-holder). I have attached the now correct version of the two initial plots posted to this ticket:
So, we see that the difference between the default and obs_subaru fluxMin settings generally does not have much impact. This is due to the detection thresholds that are in play for the icSrc catalog are set to:
detection.thresholdValue = 5.0 |
detection.includeThresholdMultiplier = 10.0 |
This also jives with the fact that there appears a cutoff roughly at S/N~60 for the sources in these catalogs (the odd lower S/N source does sneak in, and there are typically a few negative flux detections). As such, I am now going to advocate (likely on DM-32624) that the default signalToNoiseMin be set to 50 (instead of 20, and not 60 as the LSSTCam-imSim data have a cutoff much closer to 50, although for real data we perhaps want to go a bit higher) to better match the detection settings above.
Can I suggest using the ScienceSourceSelectorTask ?