# Nominal PSF required by processCcd

## Details

• Type: Bug
• Status: Done
• Priority: Major
• Resolution: Done
• Fix Version/s: None
• Component/s:
• Labels:
• 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.

## Activity

Hide

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 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
Joshua Meyers added a comment -

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
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
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
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:
Joshua Meyers
Reporter:
Merlin Fisher-Levine
Reviewers:
Watchers:
John Swinbank, Joshua Meyers, Merlin Fisher-Levine, Yusra AlSayyad