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

Fix setting of calib_psf_candidate flag to match docstring description

    XMLWordPrintable

    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

          blocks

          Story - Created by Jira Software - do not edit or delete. Issue type for a user story. DM-15649 Add a test to check the logic of the calib_psf_* flag setting

          • Done

            Activity

            Ascending order - Click to sort in descending order
            Hide
            Permalink
            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
            Permalink
            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
            Permalink
            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:

                  Jenkins

                  No builds found.