Details

    • Templates:
    • Story Points:
      2
    • Sprint:
      DRP S17-2
    • Team:
      Data Release Production

      Description

      After the processCcd refactor, non-sky images, or, in general, images for which an initial PSF estimate cannot be made, cause ProcessCcdTask to fail if config.doCalibrate=True.

      To work around this, I put a dirty hack at line 157 of my processCcd.py:

      from lsst.meas.algorithms.installGaussianPsf import InstallGaussianPsfTask, FwhmPerSigma
      task = InstallGaussianPsfTask()
      task.config.fwhm = 1.0 * 3.53223006755
      task.run(exposure=exposure)
      measFwhm = exposure.getPsf().computeShape().getDeterminantRadius() * FwhmPerSigma
      

      to ensure that there would always be kind of PSF. This should be redone in a way that someone who knows what they're doing approves of.

        Issue Links

          Activity

          Hide
          yusra Yusra AlSayyad added a comment -

          Note to the future reader who stumbles on this ticket: this ticket does not incorporate the "dirty hack" but rather the "way that someone who knows what they're doing approves of."

          REVIEW:

          Looks good and can merged after checking ci_hsc passes, but I want to check that I understand the behavior. I'm confused on why both RuntimeErrors (in characterize and detectMeasureAndEstimatePsf) were replaced.

          Previous behavior: if doMeasurePsf=False and exposure does not have a PSF already, characterize() raises a RuntimeError and never gets to detectMeasureAndEstimatePsf(). If the PSF is impossible to measure, then no processCcd for you.

          New behavior: If doMeasurePsf=False and exposure doesn't have a PSF, characterize quietly installs a simplePsf. detectMeasureAndEstimatePsf() will not reinstall simplePsf because it already has one, and will run one iteration of detection and measurement with the simplePsf.

          Why does detectMeasureAndEstimatePsf() also now install a simplePsf if not exposure.hasPsf and not doMeasurePsf (given that characterize already installs the simplePsf)? Because there was already a double safety net to begin with?

          Also, do we want to log that the source catalog has detected/measured with a dummy PSF, so users know that the source catalogs are not to be trusted? Something like:
          self.log.warn("Source catalog detected and measured with placeholder or default PSF")

          My interpretation of the 4 scenarios:
          A: doMeasurePsf=True
          1) detectMeasureAndEstimatePsf iterates until convergence. Final detection and measurement with best PSF.
          2) First iteration of detectMeasureAndEstimatePsf fails (e.g. because too few stars) <-- Raises error or final detection and measurement with dummy PSF?

          B: doMeasurePsf=False
          1) exposure has a PSF: run detect and measure with original PSF
          2) exposure does not have PSF: run detect and measure with dummy PSF <--I might like a warning that this has happened.

          Show
          yusra Yusra AlSayyad added a comment - Note to the future reader who stumbles on this ticket: this ticket does not incorporate the "dirty hack" but rather the "way that someone who knows what they're doing approves of." REVIEW: Looks good and can merged after checking ci_hsc passes, but I want to check that I understand the behavior. I'm confused on why both RuntimeErrors (in characterize and detectMeasureAndEstimatePsf) were replaced. Previous behavior : if doMeasurePsf=False and exposure does not have a PSF already, characterize() raises a RuntimeError and never gets to detectMeasureAndEstimatePsf() . If the PSF is impossible to measure, then no processCcd for you. New behavior : If doMeasurePsf=False and exposure doesn't have a PSF, characterize quietly installs a simplePsf. detectMeasureAndEstimatePsf() will not reinstall simplePsf because it already has one, and will run one iteration of detection and measurement with the simplePsf. Why does detectMeasureAndEstimatePsf() also now install a simplePsf if not exposure.hasPsf and not doMeasurePsf (given that characterize already installs the simplePsf)? Because there was already a double safety net to begin with? Also, do we want to log that the source catalog has detected/measured with a dummy PSF, so users know that the source catalogs are not to be trusted? Something like: self.log.warn("Source catalog detected and measured with placeholder or default PSF") My interpretation of the 4 scenarios: A: doMeasurePsf=True 1) detectMeasureAndEstimatePsf iterates until convergence. Final detection and measurement with best PSF. 2) First iteration of detectMeasureAndEstimatePsf fails (e.g. because too few stars) <-- Raises error or final detection and measurement with dummy PSF? B: doMeasurePsf=False 1) exposure has a PSF: run detect and measure with original PSF 2) exposure does not have PSF: run detect and measure with dummy PSF <--I might like a warning that this has happened.
          Hide
          jmeyers314 Joshua Meyers added a comment -

          Thanks for the careful reading Yusra AlSayyad.

          Yes, I think the previous double safety net implies that we should replace the RuntimeError with installSimplePsf() in both places, in case detectMeasureAndEstimatePsf() ever got called outside of characterize(), for instance.

          The logging suggestion is a good idea. I'll add that.

          I'm not sure about your last point ("First iteration of detectMeasureAndEstimatePsf fails ...") Presumably if detectMeasureAndEstimatePsf failed it would raise an unhandled exception and stop right there? I wouldn't expect execution to continue to final detection and measurement with the dummy PSF, but maybe I missed something?

          Show
          jmeyers314 Joshua Meyers added a comment - Thanks for the careful reading Yusra AlSayyad . Yes, I think the previous double safety net implies that we should replace the RuntimeError with installSimplePsf() in both places, in case detectMeasureAndEstimatePsf() ever got called outside of characterize() , for instance. The logging suggestion is a good idea. I'll add that. I'm not sure about your last point ("First iteration of detectMeasureAndEstimatePsf fails ...") Presumably if detectMeasureAndEstimatePsf failed it would raise an unhandled exception and stop right there? I wouldn't expect execution to continue to final detection and measurement with the dummy PSF, but maybe I missed something?
          Hide
          jmeyers314 Joshua Meyers added a comment -

          For posterity: Yusra AlSayyad and I agreed that the last question above is resolved by the fact that this ticket doesn't change the behavior when doMeasurePsf=True and detectMeasureAndEstimatePsf fails.

          Show
          jmeyers314 Joshua Meyers added a comment - For posterity: Yusra AlSayyad and I agreed that the last question above is resolved by the fact that this ticket doesn't change the behavior when doMeasurePsf=True and detectMeasureAndEstimatePsf fails.

            People

            • Assignee:
              jmeyers314 Joshua Meyers
              Reporter:
              mfisherlevine Merlin Fisher-Levine
              Reviewers:
              Yusra AlSayyad
              Watchers:
              John Swinbank, Joshua Meyers, Merlin Fisher-Levine, Yusra AlSayyad
            • Votes:
              0 Vote for this issue
              Watchers:
              4 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development

                  Agile