From c933b079832d6ccce426b4fffbf939b8254524b1 Mon Sep 17 00:00:00 2001 From: Ian Sullivan Date: Thu, 14 Nov 2024 23:00:09 -0800 Subject: [PATCH 1/3] Remove deprecated NaiveCentroid --- include/lsst/ip/diffim/DipoleAlgorithms.h | 82 --------- python/lsst/ip/diffim/__init__.py | 5 - python/lsst/ip/diffim/dipoleAlgorithms.cc | 39 ----- src/DipoleAlgorithms.cc | 203 ---------------------- tests/test_dipole.py | 35 ---- 5 files changed, 364 deletions(-) diff --git a/include/lsst/ip/diffim/DipoleAlgorithms.h b/include/lsst/ip/diffim/DipoleAlgorithms.h index a907d6378..69b3a9db1 100644 --- a/include/lsst/ip/diffim/DipoleAlgorithms.h +++ b/include/lsst/ip/diffim/DipoleAlgorithms.h @@ -204,88 +204,6 @@ inline DipoleFluxAlgorithm::DipoleFluxAlgorithm( _negativeKeys = ResultKey(schema[name+"_neg"]); } -/* -class that knows how to calculate centroids as a simple unweighted first - * moment of the 3x3 region around the peaks - */ -class [[deprecated( - "This algorithm is deprecated and will be removed after v28.")]] - NaiveDipoleFlux : public DipoleFluxAlgorithm { -public: - - typedef DipoleFluxControl Control; - - NaiveDipoleFlux(Control const & ctrl, std::string const & name, afw::table::Schema & schema) : - DipoleFluxAlgorithm(ctrl, name, schema, "raw flux counts"), - _numPositiveKey(schema.addField(name+"_npos", "number of positive pixels", "count")), - _numNegativeKey(schema.addField(name+"_nneg", "number of negative pixels", "count")) - { - } - - void measure( - afw::table::SourceRecord & measRecord, - afw::image::Exposure const & exposure - ) const; - - void fail( - afw::table::SourceRecord & measRecord, - meas::base::MeasurementError * error=NULL - ) const; - -private: - - Control _ctrl; - afw::table::Key _numPositiveKey; - afw::table::Key _numNegativeKey; -}; - -/** - * @brief Intermediate base class for algorithms that compute a centroid. - */ -class [[deprecated( - "This algorithm is deprecated and will be removed after v28.")]] - NaiveDipoleCentroid : public DipoleCentroidAlgorithm { -public: - - NaiveDipoleCentroid(Control const & ctrl, std::string const & name, afw::table::Schema & schema); - /** - * @brief Tuple type that holds the keys that define a standard centroid algorithm. - * - * Algorithms are encouraged to add additional flags as appropriate, but these are required. - */ - typedef meas::base::CentroidResultKey ResultKey; - - /// @brief Return the standard centroid keys registered by this algorithm. - ResultKey const & getCenterKeys() const { return _centerKeys; } - ResultKey const & getPositiveKeys() const { return _positiveKeys; } - ResultKey const & getNegativeKeys() const { return _negativeKeys; } - - void measure( - afw::table::SourceRecord & measRecord, - afw::image::Exposure const & exposure - ) const; - - void mergeCentroids(afw::table::SourceRecord & source, double posValue, double negValue) const; - - void fail( - afw::table::SourceRecord & measRecord, - meas::base::MeasurementError * error=NULL - ) const; - -protected: - /// @brief Initialize with a manually-constructed key tuple. - NaiveDipoleCentroid(Control const & ctrl, std::string const & name, afw::table::Schema & schema, - ResultKey const & positiveKeys, ResultKey const & negativeKeys); - -private: - - Control _ctrl; - meas::base::FluxResultKey _fluxResultKey; - meas::base::FlagHandler _flagHandler; -}; - - - /** * Implementation of Psf dipole flux diff --git a/python/lsst/ip/diffim/__init__.py b/python/lsst/ip/diffim/__init__.py index a6173ce18..58f3879c4 100755 --- a/python/lsst/ip/diffim/__init__.py +++ b/python/lsst/ip/diffim/__init__.py @@ -43,9 +43,4 @@ # automatically register ip_diffim Algorithms; # CENTROID_ORDER=0.0, FLUX_ORDER==2.0 from lsst.meas.base import wrapSimpleAlgorithm -wrapSimpleAlgorithm(NaiveDipoleCentroid, Control=DipoleCentroidControl, executionOrder=0.0, - deprecated="Plugin 'NaiveDipoleCentroid' is deprecated and will be removed after v28.0") -wrapSimpleAlgorithm(NaiveDipoleFlux, Control=DipoleFluxControl, executionOrder=2.0, - deprecated="Plugin 'NaiveDipoleFlux' is deprecated and will be removed after v28.0") wrapSimpleAlgorithm(PsfDipoleFlux, Control=PsfDipoleFluxControl, executionOrder=2.0) - diff --git a/python/lsst/ip/diffim/dipoleAlgorithms.cc b/python/lsst/ip/diffim/dipoleAlgorithms.cc index bb94d6fbd..fed4f7a15 100644 --- a/python/lsst/ip/diffim/dipoleAlgorithms.cc +++ b/python/lsst/ip/diffim/dipoleAlgorithms.cc @@ -102,39 +102,6 @@ void declareDipoleFluxAlgorithm(lsst::cpputils::python::WrapperCollection &wrapp }); } -// Hide deprecation warnings when building pybind11. -// Remove these pragmas on DM-44030 -#pragma GCC diagnostic push -#pragma GCC diagnostic ignored "-Wdeprecated" -void declareNaiveDipoleFlux(lsst::cpputils::python::WrapperCollection &wrappers) { - using PyNaiveDipoleFlux = py::class_, DipoleFluxAlgorithm>; - - wrappers.wrapType(PyNaiveDipoleFlux(wrappers.module, "NaiveDipoleFlux"), [](auto &mod, auto &cls) { - cls.def(py::init(), "ctrl"_a, - "name"_a, "schema"_a); - - cls.def("measure", &NaiveDipoleFlux::measure, "measRecord"_a, "exposure"_a); - cls.def("fail", &NaiveDipoleFlux::fail, "measRecord"_a, "error"_a = nullptr); - }); -} - -void declareNaiveDipoleCentroid(lsst::cpputils::python::WrapperCollection &wrappers) { - using PyNaiveDipoleCentroid = - py::class_, DipoleCentroidAlgorithm>; - wrappers.wrapType(PyNaiveDipoleCentroid(wrappers.module, "NaiveDipoleCentroid"), [](auto &mod, auto &cls) { - cls.def(py::init(), - "ctrl"_a, "name"_a, "schema"_a); - - cls.def("getCenterKeys", &NaiveDipoleCentroid::getCenterKeys); - cls.def("getPositiveKeys", &NaiveDipoleCentroid::getPositiveKeys); - cls.def("getNegativeKeys", &NaiveDipoleCentroid::getNegativeKeys); - - cls.def("measure", &NaiveDipoleCentroid::measure, "measRecord"_a, "exposure"_a); - cls.def("mergeCentroids", &NaiveDipoleCentroid::mergeCentroids, "source"_a, "posValue"_a, "negValue"_a); - cls.def("fail", &NaiveDipoleCentroid::fail, "measRecord"_a, "error"_a = nullptr); - }); -} - void declarePsfDipoleFlux(lsst::cpputils::python::WrapperCollection &wrappers) { using PyPsfDipoleFlux = py::class_, DipoleFluxAlgorithm>; @@ -157,15 +124,9 @@ void wrapDipoleAlgorithms(lsst::cpputils::python::WrapperCollection &wrappers) { declareDipolePsfFluxControl(wrappers); declareDipoleCentroidAlgorithm(wrappers); declareDipoleFluxAlgorithm(wrappers); - declareNaiveDipoleFlux(wrappers); - declareNaiveDipoleCentroid(wrappers); declarePsfDipoleFlux(wrappers); } -// Stop hiding deprecation warnings when building pybind11. -// Remove these pragmas on DM-44030 -#pragma GCC diagnostic pop - } // diffim } // ip } // lsst diff --git a/src/DipoleAlgorithms.cc b/src/DipoleAlgorithms.cc index 4bc6f762f..e4bb06ab0 100644 --- a/src/DipoleAlgorithms.cc +++ b/src/DipoleAlgorithms.cc @@ -86,209 +86,6 @@ meas::base::FlagDefinitionList const & DipoleCentroidAlgorithm::getFlagDefinitio int const POSFLUXPAR(5); // Parameter for the flux of the positive lobe -namespace { - -void naiveCentroid( - afw::table::SourceRecord & source, - afw::image::Exposure const& exposure, - geom::Point2I const & center, - meas::base::CentroidResultKey const & keys - ) -{ - typedef afw::image::Image ImageT; - ImageT const& image = *exposure.getMaskedImage().getImage(); - // set to the input centroid, just in case all else fails - source.set(keys.getX(), center.getX()); - source.set(keys.getY(), center.getY()); - - int x = center.getX() - image.getX0(); - int y = center.getY() - image.getY0(); - - if (x < 1 || x >= image.getWidth() - 1 || y < 1 || y >= image.getHeight() - 1) { - throw LSST_EXCEPT(pex::exceptions::LengthError, - (boost::format("Object at (%d, %d) is too close to the edge") - % x % y).str()); - } - - ImageT::xy_locator im = image.xy_at(x, y); - - double const sum = - (im(-1, 1) + im( 0, 1) + im( 1, 1) + - im(-1, 0) + im( 0, 0) + im( 1, 0) + - im(-1, -1) + im( 0, -1) + im( 1, -1)); - - - if (sum == 0.0) { - throw LSST_EXCEPT(pexExceptions::RuntimeError, - (boost::format("Object at (%d, %d) has no counts") % - x % y).str()); - } - - double const sum_x = - -im(-1, 1) + im( 1, 1) + - -im(-1, 0) + im( 1, 0) + - -im(-1, -1) + im( 1, -1); - double const sum_y = - (im(-1, 1) + im( 0, 1) + im( 1, 1)) - - (im(-1, -1) + im( 0, -1) + im( 1, -1)); - - float xx = afw::image::indexToPosition(x + image.getX0()) + sum_x / sum; - float yy = afw::image::indexToPosition(y + image.getY0()) + sum_y / sum; - source.set(keys.getX(), xx); - source.set(keys.getY(), yy); -} - -} // anonymous namespace - - -NaiveDipoleCentroid::NaiveDipoleCentroid( - Control const & ctrl, - std::string const & name, - afw::table::Schema & schema -) : DipoleCentroidAlgorithm(ctrl, name, schema, "unweighted first moment centroid"), - _ctrl(ctrl) -{ } - -/** - * Given an image and a pixel position, return a Centroid using a naive 3x3 weighted moment - */ -void NaiveDipoleCentroid::measure( - afw::table::SourceRecord & source, - afw::image::Exposure const & exposure -) const { - afw::detection::PeakCatalog const& peaks = source.getFootprint()->getPeaks(); - - int posInd = 0; - double posValue = peaks[posInd].getPeakValue(), negValue = 0; - if (posValue < 0.) { /* All peaks are negative so use the *most* negative value */ - posInd = peaks.size() - 1; - posValue = peaks[posInd].getPeakValue(); - } - naiveCentroid(source, exposure, peaks[posInd].getI(), - (posValue >= 0 ? getPositiveKeys() : getNegativeKeys())); - - if (posValue > 0. && posInd == 0 && peaks.size() > 1) { /* See if there's also a negative peak */ - int negInd = peaks.size() - 1; - negValue = peaks[negInd].getPeakValue(); - if (posValue > 0. && negValue < 0.) { - naiveCentroid(source, exposure, peaks[negInd].getI(), - (negValue >= 0 ? getPositiveKeys() : getNegativeKeys())); - } - } - - mergeCentroids(source, posValue, negValue); - -} - -void NaiveDipoleCentroid::mergeCentroids(afw::table::SourceRecord & source, - double posValue, double negValue) const { - - double pos_x, pos_y, pos_f; - double neg_x, neg_y, neg_f; - - pos_x = source.get(getPositiveKeys().getX()); - pos_y = source.get(getPositiveKeys().getY()); - pos_f = posValue; - - neg_x = source.get(getNegativeKeys().getX()); - neg_y = source.get(getNegativeKeys().getY()); - neg_f = -negValue; - - if(std::isfinite(pos_x) && std::isfinite(pos_y) && - std::isfinite(neg_x) && std::isfinite(neg_y)) { - source.set(getCenterKeys().getX(), (pos_x * pos_f + neg_x * neg_f) / (pos_f + neg_f)); - source.set(getCenterKeys().getY(), (pos_y * pos_f + neg_y * neg_f) / (pos_f + neg_f)); - } else if (std::isfinite(pos_x) && std::isfinite(pos_y)) { - source.set(getCenterKeys().getX(), pos_x); - source.set(getCenterKeys().getY(), pos_y); - } else { - source.set(getCenterKeys().getX(), neg_x); - source.set(getCenterKeys().getY(), neg_y); - } -} - -void NaiveDipoleCentroid::fail(afw::table::SourceRecord & measRecord, - meas::base::MeasurementError * error) const { - _flagHandler.handleFailure(measRecord, error); -} - - -namespace { - -class NaiveDipoleFootprinter { -public: - explicit NaiveDipoleFootprinter() : _sumPositive(0.0), _sumNegative(0.0), _numPositive(0), - _numNegative(0) {} - - /// Reset everything for a new Footprint - void reset() { - _sumPositive = _sumNegative = 0.0; - _numPositive = _numNegative = 0; - } - void reset(afwDet::Footprint const&) {} - - /// method called for each pixel by applyFunctor - void operator() (geom::Point2I const & pos, - afw::image::MaskedImage::Image::Pixel const & ival, - afw::image::MaskedImage::Image::Pixel const & vval) { - if (ival >= 0.0) { - _sumPositive += ival; - _varPositive += vval; - ++_numPositive; - } else { - _sumNegative += ival; - _varPositive += vval; - ++_numNegative; - } - } - - double getSumPositive() const { return _sumPositive; } - double getSumNegative() const { return _sumNegative; } - double getVarPositive() const { return _sumPositive; } - double getVarNegative() const { return _sumNegative; } - int getNumPositive() const { return _numPositive; } - int getNumNegative() const { return _numNegative; } - -private: - double _sumPositive; - double _sumNegative; - double _varPositive; - double _varNegative; - int _numPositive; - int _numNegative; -}; - -} // anonymous namespace - - -/** - * Given an image and a pixel position, return a Centroid using a naive 3x3 weighted moment - */ -void NaiveDipoleFlux::measure( - afw::table::SourceRecord & source, - afw::image::Exposure const & exposure -) const { - typedef afw::image::Exposure::MaskedImageT MaskedImageT; - - NaiveDipoleFootprinter functor; - source.getFootprint()->getSpans()->applyFunctor(functor, *(exposure.getMaskedImage().getImage()), - *(exposure.getMaskedImage().getVariance())); - - source.set(getPositiveKeys().getInstFlux(), functor.getSumPositive()); - source.set(getPositiveKeys().getInstFluxErr(), ::sqrt(functor.getVarPositive())); - source.set(_numPositiveKey, functor.getNumPositive()); - - source.set(getNegativeKeys().getInstFlux(), functor.getSumNegative()); - source.set(getNegativeKeys().getInstFluxErr(), ::sqrt(functor.getVarNegative())); - source.set(_numNegativeKey, functor.getNumNegative()); - functor.reset(); -} - -void NaiveDipoleFlux::fail(afw::table::SourceRecord & measRecord, meas::base::MeasurementError * error) const { - _flagHandler.handleFailure(measRecord, error); -} - - /** * Class to minimize PsfDipoleFlux; this is the object that Minuit minimizes */ diff --git a/tests/test_dipole.py b/tests/test_dipole.py index c029729ce..bca41af07 100644 --- a/tests/test_dipole.py +++ b/tests/test_dipole.py @@ -131,41 +131,6 @@ def setUp(self): self.w, self.h = 100, 100 # size of image self.xc, self.yc = 50, 50 # location of center of dipole - # Remove this test on DM-44030 - def testNaiveDipoleCentroid(self): - control = ipDiffim.DipoleCentroidControl() - psf, psfSum, exposure, s = createDipole(self.w, self.h, self.xc, self.yc) - plugin, cat = makePluginAndCat(ipDiffim.NaiveDipoleCentroid, "test", control, centroid="centroid") - source = cat.addNew() - source.set("centroid_x", 50) - source.set("centroid_y", 50) - source.setFootprint(s.getFootprint()) - plugin.measure(source, exposure) - for key in ("_pos_x", "_pos_y", "_pos_xErr", "_pos_yErr", "_pos_flag", - "_neg_x", "_neg_y", "_neg_xErr", "_neg_yErr", "_neg_flag"): - try: - source.get("test" + key) - except Exception: - self.fail() - - # Remove this test on DM-44030 - def testNaiveDipoleFluxControl(self): - psf, psfSum, exposure, s = createDipole(self.w, self.h, self.xc, self.yc) - control = ipDiffim.DipoleFluxControl() - psf, psfSum, exposure, s = createDipole(self.w, self.h, self.xc, self.yc) - plugin, cat = makePluginAndCat(ipDiffim.NaiveDipoleFlux, "test", control, centroid="centroid") - source = cat.addNew() - source.set("centroid_x", 50) - source.set("centroid_y", 50) - source.setFootprint(s.getFootprint()) - plugin.measure(source, exposure) - for key in ("_pos_instFlux", "_pos_instFluxErr", "_pos_flag", "_npos", - "_neg_instFlux", "_neg_instFluxErr", "_neg_flag", "_nneg"): - try: - source.get("test" + key) - except Exception: - self.fail() - def testPsfDipoleFluxControl(self): psf, psfSum, exposure, s = createDipole(self.w, self.h, self.xc, self.yc) psf, psfSum, exposure, s = createDipole(self.w, self.h, self.xc, self.yc) From 592d5566a30921829642c264df6676c9393c2df7 Mon Sep 17 00:00:00 2001 From: Ian Sullivan Date: Thu, 14 Nov 2024 23:01:07 -0800 Subject: [PATCH 2/3] Remove deprecated badSourceFlags --- python/lsst/ip/diffim/subtractImages.py | 13 ------------- 1 file changed, 13 deletions(-) diff --git a/python/lsst/ip/diffim/subtractImages.py b/python/lsst/ip/diffim/subtractImages.py index c5d6fdf8e..25b901439 100644 --- a/python/lsst/ip/diffim/subtractImages.py +++ b/python/lsst/ip/diffim/subtractImages.py @@ -212,19 +212,6 @@ class AlardLuptonSubtractBaseConfig(lsst.pex.config.Config): default=3, doc="Minimum number of sources needed for calculating the PSF matching kernel." ) - badSourceFlags = lsst.pex.config.ListField( - dtype=str, - doc="Flags that, if set, the associated source should not " - "be used to determine the PSF matching kernel.", - default=("sky_source", "slot_Centroid_flag", - "slot_ApFlux_flag", "slot_PsfFlux_flag", - "base_PixelFlags_flag_interpolated", - "base_PixelFlags_flag_saturated", - "base_PixelFlags_flag_bad", - ), - deprecated="This field is no longer used and will be removed after v28." - "Set the equivalent field for the sourceSelector subtask instead.", - ) excludeMaskPlanes = lsst.pex.config.ListField( dtype=str, default=("NO_DATA", "BAD", "SAT", "EDGE", "FAKE"), From 216948707df64a5a52f29ee4bb863bd49af6b2d6 Mon Sep 17 00:00:00 2001 From: Ian Sullivan Date: Thu, 14 Nov 2024 23:27:24 -0800 Subject: [PATCH 3/3] Update dipole fit documentation. --- .../tasks/lsst.ip.diffim.DipoleMeasurementTask.rst | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/doc/lsst.ip.diffim/tasks/lsst.ip.diffim.DipoleMeasurementTask.rst b/doc/lsst.ip.diffim/tasks/lsst.ip.diffim.DipoleMeasurementTask.rst index c8d21f569..f2b2f057f 100644 --- a/doc/lsst.ip.diffim/tasks/lsst.ip.diffim.DipoleMeasurementTask.rst +++ b/doc/lsst.ip.diffim/tasks/lsst.ip.diffim.DipoleMeasurementTask.rst @@ -19,12 +19,9 @@ Their respective algorithm control classes are defined in DipoleCentroidControl and DipoleFluxControl. Each centroid and flux measurement will have _neg (negative) and _pos (positive lobe) fields. -The first set of measurements uses a "naive" algorithm for centroid and flux -measurements, implemented in NaiveDipoleCentroidControl and -NaiveDipoleFluxControl. The algorithm uses a naive 3x3 weighted moment around -the nominal centroids of each peak in the Source Footprint. These algorithms -fill the table fields ip_diffim_NaiveDipoleCentroid* and -ip_diffim_NaiveDipoleFlux* +The first set of measurements uses the SDSS algorithm for centroid and flux +measurements. +This centroid will be used as a fallback, if the below dipole fit fails or if the source does not have a positive and negative peak to attempt a dipole fit to. The second set of measurements undertakes a joint-Psf model on the negative and positive lobe simultaneously. This fit simultaneously solves for the negative