Details
-
Type:
Bug
-
Status: Done
-
Resolution: Done
-
Fix Version/s: None
-
Component/s: meas_base
-
Labels:None
-
Story Points:4
-
Epic Link:
-
Team:Architecture
-
Urgent?:No
Description
Running the following snippet causes the Python kernel crashes:
import lsst.afw.geom as afwGeom |
import lsst.afw.image as afwImage |
import lsst.meas.base as measBase |
import numpy as np |
|
|
img = afwImage.ImageD(np.random.randn(16,16)) |
shape = afwGeom.Quadrupole(-2, -2, 0) # <-- This is invalid |
flux = measBase.SdssShapeAlgorithm.computeFixedMomentsFlux(img, shape, |
img.getBBox().getCenter())
|
|
This is yet another example of a user error that causes a complete crash instead of raising Exceptions. The correct way is to set `normalize=True` when constructing the `shape` variable.
Attachments
Issue Links
- relates to
-
DM-3476 calcmom in SdssShape.cc has poor error reporting
- Won't Fix
Activity
Output of test for the fixed code:
Traceback (most recent call last):
|
File "test.py", line 15, in <module>
|
flux = measBase.SdssShapeAlgorithm.computeFixedMomentsFlux(img, shape,
|
lsst.pex.exceptions.wrappers.RuntimeError:
|
File "src/SdssShape.cc", line 840, in static lsst::meas::base::FluxResult lsst::meas::base::SdssShapeAlgorithm::computeFixedMomentsFlux(const ImageT&, const lsst::afw::geom::ellipses::Quadrupole&, const Point2D&) [with ImageT = lsst::afw::image::Image<double>; lsst::geom::Point2D = lsst::geom::Point<double, 2>]
|
Error from calcmom {0}
|
lsst::pex::exceptions::RuntimeError: 'Error from calcmom'
|
Matthias Wittgen
added a comment - Output of test for the fixed code:
Traceback (most recent call last):
File "test.py", line 15, in <module>
flux = measBase.SdssShapeAlgorithm.computeFixedMomentsFlux(img, shape,
lsst.pex.exceptions.wrappers.RuntimeError:
File "src/SdssShape.cc", line 840, in static lsst::meas::base::FluxResult lsst::meas::base::SdssShapeAlgorithm::computeFixedMomentsFlux(const ImageT&, const lsst::afw::geom::ellipses::Quadrupole&, const Point2D&) [with ImageT = lsst::afw::image::Image<double>; lsst::geom::Point2D = lsst::geom::Point<double, 2>]
Error from calcmom {0}
lsst::pex::exceptions::RuntimeError: 'Error from calcmom'
The changes appear sufficient to not cause Python to crash, but the exceptions are not consistent. It should not be RuntimeError that's thrown, but InvalidParameterError, as in say here.] In other words, the exception must be consistent with this test code:
import lsst.afw.geom as afwGeom |
import lsst.afw.image as afwImage |
import lsst.meas.base as measBase |
import numpy as np |
|
img = afwImage.ImageD(np.random.randn(16,16)) |
shape = afwGeom.Quadrupole(2, -2, 0) # <-- This is also invalid |
flux = measBase.SdssShapeAlgorithm.computeFixedMomentsFlux(img, shape,
|
img.getBBox().getCenter())
|
Yes, that's a result of `calcmon` returning a C style error flag. Better throw the correct exception at the parameter check.
Matthias Wittgen
added a comment - Yes, that's a result of `calcmon` returning a C style error flag. Better throw the correct exception at the parameter check. cleaned up `calcmon`
Matthias Wittgen
added a comment - cleaned up `calcmon` Throws now
Traceback (most recent call last):
|
File "test.py", line 15, in <module>
|
flux = measBase.SdssShapeAlgorithm.computeFixedMomentsFlux(img, shape,
|
lsst.pex.exceptions.wrappers.InvalidParameterError:
|
File "src/SdssShape.cc", line 208, in int lsst::meas::base::{anonymous}::calcmom(const ImageT&, float, float, const BoxI&, float, bool, double, double, double, double&) [with ImageT = lsst::afw::image::Image<double>; lsst::geom::BoxI = lsst::geom::Box2I]
|
invalid weight parameter(s) {0}
|
|
Matthias Wittgen
added a comment - - edited Throws now
Traceback (most recent call last):
File "test.py", line 15, in <module>
flux = measBase.SdssShapeAlgorithm.computeFixedMomentsFlux(img, shape,
lsst.pex.exceptions.wrappers.InvalidParameterError:
File "src/SdssShape.cc", line 208, in int lsst::meas::base::{anonymous}::calcmom(const ImageT&, float, float, const BoxI&, float, bool, double, double, double, double&) [with ImageT = lsst::afw::image::Image<double>; lsst::geom::BoxI = lsst::geom::Box2I]
invalid weight parameter(s) {0}
Thanks Matthias. I was curious about some of the other changes introduced. Otherwise the changes look good. The commit has to be split up into at least two - one with just the cleaning up, one fixing the crash.
Made the changes to have two commits.
Passes CI pipeline. https://ci.lsst.codes/job/stack-os-matrix/36356/display/redirect
Matthias Wittgen
added a comment - Made the changes to have two commits.
Passes CI pipeline. https://ci.lsst.codes/job/stack-os-matrix/36356/display/redirect 
Assert makes no sense in this context `int calcmom` already gracefullt checks on the input parameters.