DM-32285: Fix yy <-> xy bug in SdssShape uncertainties #199
Conversation
|
The convention adopted in |
| // convention in afw::geom::ellipses is to order moments (xx, yy, xy), | ||
| // but the older algorithmic code uses (xx, xy, yy) - the order of | ||
| // indices here is not a bug. | ||
| // convention is to order moments (xx, yy, xy) |
There was a problem hiding this comment.
Looks good, but can you smash the first two commits so there isn't a commit with the code change but not the doc change?
tests/test_SdssShape.py
Outdated
| @@ -129,6 +139,47 @@ def testMeasureBadPsf(self): | |||
| self._checkShape(result, record) | |||
| self.assertTrue(result.getFlag(lsst.meas.base.SdssShapeAlgorithm.PSF_SHAPE_BAD.number)) | |||
|
|
|||
| def testMonteCarlo(self): | |||
| """Test an ideal simulation, with no noise. | |||
There was a problem hiding this comment.
"with no noise" doesn't seem right. Maybe "with known noise" or "with controlled noise"?
There was a problem hiding this comment.
This was just copy-pasted from other MonteCarlo tests. But you're right that this is not accurate and will change.
There was a problem hiding this comment.
Changed it to "with deterministic noise"
tests/test_SdssShape.py
Outdated
|
|
||
| Demonstrate that: | ||
|
|
||
| - We get exactly the right answer, and |
tests/test_SdssShape.py
Outdated
| for suffix in ["xx", "yy", "xy"]: | ||
| shapeMean = np.mean(catalog["base_SdssShape_"+suffix]) | ||
| shapeErrMean = np.nanmean(catalog["base_SdssShape_"+suffix+"Err"]) | ||
| shapeStandardDeviation = 0.5*(np.nanpercentile(catalog["base_SdssShape_"+suffix], 84) |
There was a problem hiding this comment.
Maybe call this shapeInterval68? I think it's only equivalent to the standard deviation for a Gaussian distribution.
arunkannawadi
force-pushed
the
tickets/DM-32285
branch
from
2b0db86
to
cc403e8
Compare
Due to a convention mismatch, the
_yyErrand_xyErrcomponents are interchanged forbase_SdssShapeplugin. This PR fixes this bug and adds a MonteCarlo unit test that would have caught it in the first place.