Uploaded image for project: 'Data Management'
  1. Data Management
  2. DM-32624

Override source selection criteria in SFM calibration stages for LSSTCam-imSim

    XMLWordPrintable

    Details

    • 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

            Hide
            yusra Yusra AlSayyad added a comment -

            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 threshold
            self.measurePsf.starSelector["objectSize"].signalToNoiseMin = 50
            

            right under where the detection threshold is set to 50 in the characterizeImageConfig.setDefaults:

                self.detection.thresholdValue = 5.0
                self.detection.includeThresholdMultiplier = 10.0 
            

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

            Show
            yusra Yusra AlSayyad added a comment - 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 threshold self.measurePsf.starSelector["objectSize"].signalToNoiseMin = 50 right under where the detection threshold is set to 50 in the characterizeImageConfig.setDefaults: self.detection.thresholdValue = 5.0 self.detection.includeThresholdMultiplier = 10.0 But 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.
            Hide
            lauren Lauren MacArthur added a comment -

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

            Show
            lauren Lauren MacArthur added a comment - 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).
            Hide
            lauren Lauren MacArthur added a comment -

            Also a huge +1 from me on your "random idea. Our config fields would benefit from a defaultRationale field. One day...."!!

            Show
            lauren Lauren MacArthur added a comment - Also a huge +1 from me on your "random idea. Our config fields would benefit from a defaultRationale field. One day...."!!
            Hide
            lauren Lauren MacArthur added a comment -

            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.

            Show
            lauren Lauren MacArthur added a comment - 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.
            Hide
            lauren Lauren MacArthur added a comment -

            Thanks for reviewing the PRs, Yusra.  Backport now complete.

            Show
            lauren Lauren MacArthur added a comment - Thanks for reviewing the PRs, Yusra.  Backport now complete.

              People

              Assignee:
              lauren Lauren MacArthur
              Reporter:
              lauren Lauren MacArthur
              Reviewers:
              Yusra AlSayyad
              Watchers:
              Colin Slater, Eli Rykoff, Heather Kelly, James Chiang, Jim Bosch, Lauren MacArthur, Yusra AlSayyad
              Votes:
              0 Vote for this issue
              Watchers:
              7 Start watching this issue

                Dates

                Created:
                Updated:
                Resolved:

                  Jenkins Builds

                  No builds found.