-
Notifications
You must be signed in to change notification settings - Fork 10
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Tickets/dm 8108 #79
Tickets/dm 8108 #79
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,3 +1,4 @@ | ||
|
||
// -*- lsst-c++ -*- | ||
/* | ||
* LSST Data Management System | ||
|
@@ -28,9 +29,8 @@ | |
|
||
#include "lsst/afw/table/Source.h" | ||
#include "lsst/afw/detection/Psf.h" | ||
#include "lsst/afw/detection/FootprintArray.h" | ||
#include "lsst/afw/detection/FootprintArray.cc" | ||
#include "lsst/log/Log.h" | ||
#include "lsst/afw/geom/SpanSet.h" | ||
#include "lsst/meas/base/PsfFlux.h" | ||
|
||
namespace lsst { namespace meas { namespace base { | ||
|
@@ -88,7 +88,8 @@ void PsfFluxAlgorithm::measure( | |
_flagHandler.setValue(measRecord, FAILURE.number, true); // if we had a suspect flag, we'd set that instead | ||
_flagHandler.setValue(measRecord, EDGE.number, true); | ||
} | ||
afw::detection::Footprint fitRegion(fitBBox); | ||
auto fitRegionSpans = std::make_shared<afw::geom::SpanSet>(fitBBox); | ||
afw::detection::Footprint fitRegion(fitRegionSpans); | ||
if (!_ctrl.badMaskPlanes.empty()) { | ||
afw::image::MaskPixel badBits = 0x0; | ||
for ( | ||
|
@@ -98,7 +99,8 @@ void PsfFluxAlgorithm::measure( | |
) { | ||
badBits |= exposure.getMaskedImage().getMask()->getPlaneBitMask(*i); | ||
} | ||
fitRegion.intersectMask(*exposure.getMaskedImage().getMask(), badBits); | ||
fitRegion.setSpans(fitRegion.getSpans()->intersectNot(*exposure.getMaskedImage().getMask(), | ||
badBits)->clippedTo(exposure.getMaskedImage().getMask()->getBBox())); | ||
} | ||
if (fitRegion.getArea() == 0) { | ||
throw LSST_EXCEPT( | ||
|
@@ -110,25 +112,14 @@ void PsfFluxAlgorithm::measure( | |
typedef afw::detection::Psf::Pixel PsfPixel; | ||
typedef afw::image::MaskedImage<float>::Variance::Pixel VarPixel; | ||
ndarray::EigenView<PsfPixel,1,1,Eigen::ArrayXpr> model( | ||
afw::detection::flattenArray( | ||
fitRegion, | ||
psfImage->getArray(), | ||
psfImage->getXY0() | ||
) | ||
fitRegion.getSpans()->flatten(psfImage->getArray(), psfImage->getXY0()) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That's an improvement. |
||
); | ||
ndarray::EigenView<float,1,1,Eigen::ArrayXpr> data( | ||
afw::detection::flattenArray( | ||
fitRegion, | ||
exposure.getMaskedImage().getImage()->getArray(), | ||
exposure.getXY0() | ||
) | ||
fitRegion.getSpans()->flatten(exposure.getMaskedImage().getImage()->getArray(), exposure.getXY0()) | ||
); | ||
ndarray::EigenView<VarPixel,1,1,Eigen::ArrayXpr> variance( | ||
afw::detection::flattenArray( | ||
fitRegion, | ||
exposure.getMaskedImage().getVariance()->getArray(), | ||
exposure.getXY0() | ||
) | ||
fitRegion.getSpans()->flatten(exposure.getMaskedImage().getVariance()->getArray(), | ||
exposure.getXY0()) | ||
); | ||
PsfPixel alpha = model.matrix().squaredNorm(); | ||
FluxResult result; | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -257,7 +257,9 @@ def testPixelFlags(self): | |
(20, 80, ['edge']), | ||
(30, 30, ['clipped']), | ||
]: | ||
foot = afwDetection.Footprint(afwGeom.Point2I(afwGeom.Point2D(x + x0, y + y0)), 5) | ||
spans = afwGeom.SpanSet.fromShape(5).shiftedBy(x + x0, | ||
y + y0) | ||
foot = afwDetection.Footprint(spans) | ||
source = cat.makeRecord() | ||
source.setFootprint(foot) | ||
source.set("centroid_x", x+x0) | ||
|
@@ -276,7 +278,9 @@ def testPixelFlags(self): | |
source.set("centroid_x", float("NAN")) | ||
source.set("centroid_y", 40) | ||
source.set("centroid_flag", True) | ||
source.setFootprint(afwDetection.Footprint(afwGeom.Point2I(afwGeom.Point2D(x + x0, y + y0)), 5)) | ||
tmpSpanSet = afwGeom.SpanSet.fromShape(5).shiftedBy(x + x0, | ||
y + y0) | ||
source.setFootprint(afwDetection.Footprint(tmpSpanSet)) | ||
with self.assertRaises(lsst.pex.exceptions.RuntimeError): | ||
plugin.measure(source, exp) | ||
# Test that if there is no center and centroider that the object should look at the footprint | ||
|
@@ -286,7 +290,9 @@ def testPixelFlags(self): | |
with self.assertRaises(lsst.pex.exceptions.RuntimeError): | ||
plugin.measure(source, exp) | ||
# The second test will raise an error because no peaks are present | ||
source.setFootprint(afwDetection.Footprint(afwGeom.Point2I(afwGeom.Point2D(x + x0, y + y0)), 5)) | ||
tmpSpanSet2 = afwGeom.SpanSet.fromShape(5).shiftedBy(x + x0, | ||
y + y0) | ||
source.setFootprint(afwDetection.Footprint(tmpSpanSet2)) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ah the joys of 79 characters... |
||
with self.assertRaises(lsst.pex.exceptions.RuntimeError): | ||
plugin.measure(source, exp) | ||
# The final test should pass because it detects a peak, we are reusing the location of the | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -90,15 +90,19 @@ def testUndeblendedMeasurement(self): | |
parent = cat.addNew() | ||
parent.set("centroid_x", x0 + xCenter) | ||
parent.set("centroid_y", y0 + yCenter) | ||
parent.setFootprint(afwDetection.Footprint(afwGeom.Point2I(x0 + xCenter, y0 + yCenter), radius)) | ||
spanSetParent = afwGeom.SpanSet.fromShape(int(radius)) | ||
spanSetParent = spanSetParent.shiftedBy(x0 + xCenter, y0 + yCenter) | ||
parent.setFootprint(afwDetection.Footprint(spanSetParent)) | ||
|
||
# First child is bright, dominating the blend | ||
child1 = cat.addNew() | ||
child1.set("centroid_x", parent.get("centroid_x")) | ||
child1.set("centroid_y", parent.get("centroid_y")) | ||
child1.setParent(parent.getId()) | ||
image.set(xCenter, yCenter, (flux1, 0, 0)) | ||
foot1 = afwDetection.Footprint(afwGeom.Point2I(x0 + xCenter, y0 + yCenter), 0.1) | ||
spanSetChild1 = afwGeom.SpanSet.fromShape(1) | ||
spanSetChild1 = spanSetChild1.shiftedBy(x0 + xCenter, y0 + yCenter) | ||
foot1 = afwDetection.Footprint(spanSetChild1) | ||
child1.setFootprint(afwDetection.HeavyFootprintF(foot1, image)) | ||
|
||
# Second child is fainter, but we want to be able to measure it! | ||
|
@@ -107,14 +111,13 @@ def testUndeblendedMeasurement(self): | |
child2.set("centroid_y", parent.get("centroid_y") + yOffset) | ||
child2.setParent(parent.getId()) | ||
image.set(xCenter + xOffset, yCenter + yOffset, (flux2, 0, 0)) | ||
foot2 = afwDetection.Footprint(afwGeom.Point2I(x0 + xCenter + xOffset, y0 + yCenter + yOffset), 0.1) | ||
spanSetChild2 = afwGeom.SpanSet.fromShape(1) | ||
tmpPoint = (x0 + xCenter + xOffset, y0 + yCenter + yOffset) | ||
spanSetChild2 = spanSetChild2.shiftedBy(*tmpPoint) | ||
foot2 = afwDetection.Footprint(spanSetChild2) | ||
child2.setFootprint(afwDetection.HeavyFootprintF(foot2, image)) | ||
|
||
spans = [] | ||
for ss in foot1.getSpans(): | ||
spans.append(ss) | ||
for ss in foot2.getSpans(): | ||
spans.append(ss) | ||
spans = foot1.spans.union(foot2.spans) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That's an improvement. |
||
bbox = afwGeom.Box2I() | ||
bbox.include(foot1.getBBox()) | ||
bbox.include(foot2.getBBox()) | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This new syntax seems much more clumsy.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This new syntax exists for two reasons. 1. Most of this sort of work is attempted to be part of the new SpanSet class, such that in many places people can just use SpanSets, and not carry around the extra bits of footprints. I attempted to change the minimal amount in the stack to ensure maximum compatibility, and thus I did not go all in on where SpanSets could be used exclusively. Future code would probably not be used in this manor. 2. Previously the footprint functors silently dropped locations that were in the footprint but outside images, arrays, etc. This is a dangerous thing, but was commonly exploited by code that dilated the footprint such that the bounds were outside the image. For better safety it is now required that the programmer explicitly acknowledge that they wanted these pixels dropped with a clippedTo call.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, that's fair.