Details
-
Type:
Story
-
Status: Done
-
Resolution: Done
-
Fix Version/s: None
-
Component/s: obs_lsst
-
Story Points:5
-
Epic Link:
-
Team:Data Release Production
-
Urgent?:No
Description
In the analysis done on DM-16875, the suggestion was made to override the default config settings for the source selection criteria for consideration of a given source as a suitable candidate for the PSF modeling. In particular, the default setting of a minimum flux of 12500 (raw counts, I believe) seems way too high and, ideally, the cut would not be based on flux, but rather on S/N. Justification for this based on HSC data as well as the S/N thresholding functionality was added on DM-17043. [Note that the fluxMin value has long been overridden to 4000 in obs_subaru].
It seemed this change was widely agreed upon by those involved in the imsim/DC2 Run2.2i processing and was adopted – in addition to several other source selection overrides for other calibration phases – in at least some of the DESC workflows (see, e.g. the overrides in their "dc2/run2.2" branch run: https://github.com/lsst/obs_lsst/blob/dc2/run2.2/config/imsim/processCcd.py#L39-L42
and note the other overrides in that file). Unfortunately, during an investigation on PREOPS-885, it was noted that these overrides have not found their way into the master/main config files in obs_lsst, so they are not currently in effect for any processing based off the master/main configs. This situation should be rectified.
Some care may be required in the actual S/N threshold set and whether the other updates in the file pointed to above should also be adopted. In particular, I would love to hear from anyone involved the DC2 processing campaigns as to the validation of those choices (I added a few watchers as a first guess, but please do add/remove as you see fit!) I also have no idea what was used in the DP0.1 processing run, so if anyone could chime in on that, it would be greatly appreciated!
Attachments
Issue Links
Activity
I totally agree with your suggestion and was about to implement it, but it occurred to me that it may not make sense to put it there when the default is to threshold on fluxMin, not S/N. I plan to campaign to have that default changed to the latter, but that will require validating (or re-overriding) on the HSC side of things, so I think I will leave it for now and make that change if and when the default for charImage.measurePsf.starSelector["objectSize"].doSignalToNoiseLimit gets changed to true (probably along with turning charImage.measurePsf.starSelector["objectSize"].doFluxLimit to False, although they can both technically be in play).
Also a huge +1 from me on your "random idea. Our config fields would benefit from a defaultRationale field. One day...."!!
Thanks for the review, Yusra. I have merged the two branches to master. I also created the tickets/DM-32624-v23 branches according to the backporting instructions and have a Jenkins with
PRODUCTS: lsst_distrib lsst_ci ci_imsim ci_hsc
REFS: tickets/DM-32624-v23 v23.0.x v23.0.0.rc1
running. I will, of course, await the decision of the backporting board before creating the official PRs/merging.
Thanks for reviewing the PRs, Yusra. Backport now complete.
The argument for S/N =50 for ObjectSizeStarSelectorConfig makes sense warranting that CharaterizeImage is the ONLY place in the pipelines that we use ObjectSizeStarSelectorTask. This is actually the case (other than some PSF unit tests). So this is fine.
If I were doing from scratch I prob would have stuck it next to where the detection threshold is set.
# If using doSignalToNoiseLimit, SignalToNoiseMin should be >= detection thresholdself.measurePsf.starSelector["objectSize"].signalToNoiseMin = 50right under where the detection threshold is set to 50 in the characterizeImageConfig.setDefaults:
self.detection.thresholdValue = 5.0self.detection.includeThresholdMultiplier = 10.0But I haven't stared at this as long as you have, and a change in ObjectSizeStarSelectorConfig with an comment (which could be incorporated into the docstring) is fine.