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

Fix setting of calib_psf_candidate flag to match docstring description

    Details

    • Type: Bug
    • Status: Done
    • Resolution: Done
    • Fix Version/s: None
    • Component/s: pipe_tasks
    • Labels:
      None
    • Story Points:
      3
    • Epic Link:
    • Sprint:
      DRP F18-4
    • Team:
      Data Release Production

      Description

      The docstring for calib_psf_candidate reads:

                  self.candidateKey = schema.addField(
                      "calib_psf_candidate", type="Flag",
                      doc=("Flag set if the source was a candidate for PSF determination, "
                           "as determined by the star selector.")
                  )
      

      However, if the reserve fraction is set to a non-zero value, an object that was selected as a PSF candidate by the star selector but then flagged as reserved will have the calib_psf_candidate flag set to False. This is inconsistent with the docstring (and erroneously implies the object was not deemed a suitable candidate). Please update the setting of the flag to reflect its docstring description.

        Attachments

          Issue Links

            Activity

            Hide
            lauren Lauren MacArthur added a comment -

            Updated so that the calib_psf_candidate flag is true if the the star selector deemed it as such, regardless if it ends up getting used.  The two possibilities of it not being used are:

            1) specifically reserved, thus will also have the calib_psf_reserved flag set

            2) rejected by the determiner (e.g. outlier) – no special flag is set here, but the calib_psf_used flag will (obviously) not get set.

            So, we should now have:

            N_candidate >= N_reserved + N_used 
             
            where inequality would indicate some sources being rejected by the determiner, so
             
            N_rejected = N_candidate - (N_reserved + N_used)

             

            I've updated the logging to read the following:

             

            processCcd.charImage.measurePsf INFO: Measuring PSF
            processCcd.charImage.measurePsf INFO: PSF star selector found 90 candidates
            processCcd.charImage.measurePsf.reserve INFO: Reserved 18/90 sources
            processCcd.charImage.measurePsf INFO: Sending 72 candidates to PSF determiner
            ...
            processCcd.charImage.measurePsf INFO: PSF determination using 69/72 stars.
            

             

            Show
            lauren Lauren MacArthur added a comment - Updated so that the  calib_psf_candidate flag is true if the the star selector deemed it as such, regardless if it ends up getting used.  The two possibilities of it not being used are: 1) specifically reserved, thus will also have the  calib_psf_reserved flag set 2) rejected by the determiner (e.g. outlier) – no special flag is set here, but the calib_psf_used flag will (obviously) not get set. So, we should now have: N_candidate >= N_reserved + N_used   where inequality would indicate some sources being rejected by the determiner, so   N_rejected = N_candidate - (N_reserved + N_used)   I've updated the logging to read the following:   processCcd.charImage.measurePsf INFO: Measuring PSF processCcd.charImage.measurePsf INFO: PSF star selector found 90 candidates processCcd.charImage.measurePsf.reserve INFO: Reserved 18/90 sources processCcd.charImage.measurePsf INFO: Sending 72 candidates to PSF determiner ... processCcd.charImage.measurePsf INFO: PSF determination using 69/72 stars.  
            Hide
            lauren Lauren MacArthur added a comment -

            I took the confusion you expressed recently on Slack as impetus to take this up. Would you mind giving it a look?

            PR for pipe_tasks is here

            Jenkins lsst_distrib + ci_hsc passed

            Show
            lauren Lauren MacArthur added a comment - I took the confusion you expressed recently on Slack as impetus to take this up. Would you mind giving it a look? PR for pipe_tasks is here Jenkins lsst_distrib + ci_hsc passed
            Hide
            lauren Lauren MacArthur added a comment - - edited

            Thanks Bob.  I changed the variable name as you requested.  Since there was no truly descriptive name, I added a comment in the code when it is being set and details in the commit message...hopefully that'll be enough detail such that future onlookers don't get confused.

            A new Jenkins passed...merged to master.

            Show
            lauren Lauren MacArthur added a comment - - edited Thanks Bob.  I changed the variable name as you requested.  Since there was no truly descriptive name, I added a comment in the code when it is being set and details in the commit message...hopefully that'll be enough detail such that future onlookers don't get confused. A new Jenkins passed...merged to master.

              People

              • Assignee:
                lauren Lauren MacArthur
                Reporter:
                lauren Lauren MacArthur
                Reviewers:
                Bob Armstrong
                Watchers:
                Bob Armstrong, Lauren MacArthur, Robert Lupton
              • Votes:
                0 Vote for this issue
                Watchers:
                3 Start watching this issue

                Dates

                • Created:
                  Updated:
                  Resolved: