DM-52537: Raise acceptable algorithm error if WCS transform fails #433
Conversation
yalsayyad
commented
Sep 22, 2025
yalsayyad
commented
- Change AlgorithmError metadata to floats. Only ints floats and bools are allowed.
- Raise AlgorithmError from transform exception, so that it is directly caused by the transform exception instead of raising while handling the transform exception.
- Wrap computing WarpedPsf's BBox in a similar try-block because it requires evaluating the PSF which can trigger WCS transform exceptions.
fred3m
left a comment
fred3m
left a comment
There was a problem hiding this comment.
A few minor comments. I'll post a consideration for a slightly different design in JIRA.
| except pexExceptions.RangeError: | ||
| raise WarpedPsfTransformTooBigError( | ||
| f"Unable to compute the FWHM of the science Psf at {sciAvgPos}" | ||
| "due to an unexpectedly large transform." | ||
| ) |
There was a problem hiding this comment.
You don't seem to be catching this exception anymore. Is there a reason that we don't need to worry about this? Is pexExcepetions.RangeError a subset of pexExceptions.Exception?
There was a problem hiding this comment.
pexExceptions.Exception IS a catch all:
>>> import lsst.pex.exceptions as pexExceptions
>>>
>>> try:
... raise pexExceptions.RangeError("blah")
... except pexExceptions.Exception as err:
... print("caught it")
...
caught it
One of the reasons I sent this to you to review is I was wondering if you were attached to WarpedPsfTransformTooBigError specifically.
There was a problem hiding this comment.
It's used in MakePsfMatchedWarpTask to catch errors and make them annotated errors (part of DM-50702). That's a ticket that I completed but without a lot of background. Your PR catches PsfComputeShapeError too so the same failure modes should still return an annotated error, the question is do we want to persist a more informative error about why it failed or not. I have no opinion on this.
| def safeEstimateRoughShape(psf, position, jitter=2.0, maxTries=5, logger=None): | ||
| """ | ||
| Try to compute the shape of `psf` at `position`. | ||
|
|
||
| If it fails, perturb the position until success or maxTries. | ||
|
|
||
| Parameters | ||
| ---------- | ||
| psf : lsst.afw.detection.Psf | ||
| The PSF object. | ||
| position : lsst.geom.Point2D | ||
| The nominal positionition. | ||
| jitter : float | ||
| Size of random offset in pixels to try when retrying. | ||
| maxTries : int | ||
| Maximum number of attempts (including the original). |
There was a problem hiding this comment.
logger needs a docstring. Also, it might be helpful to add type hints.
- Change AlgorithmError metadata to floats. Only ints floats and bools are allowed. - Raise AlgorithmError from transform exception, so that it is directly caused by the transform exception instead of raising while handling the transform exception. - Wrap computing WarpedPsf's BBox in a similar try-block because it requires evaluating the PSF which can trigger WCS transform exceptions.
yalsayyad
force-pushed
the
tickets/DM-52537
branch
2 times, most recently
from
b3601da to
8ea1ddd
Compare
for initial rough PSF size estimate
yalsayyad
force-pushed
the
tickets/DM-52537
branch
from
8ea1ddd to
71eeae9
Compare
2 participants
