From 3c170a8fa8bcb209d6a8d7a3e1abb99fa879045c Mon Sep 17 00:00:00 2001 From: Jim Bosch Date: Tue, 16 Jun 2026 11:32:46 -0400 Subject: [PATCH 1/2] Fix bad const/shared_ptr non-usage in CoaddPsf. Const accessors should be marked as such. SkyWcs is immutable and should be passed by shared_ptr or shared_ptr (doesn't matter which) to avoid unnecessary copies. --- include/lsst/meas/algorithms/CoaddPsf.h | 23 ++++++------ .../lsst/meas/algorithms/coaddPsf/coaddPsf.cc | 6 +-- src/CoaddPsf.cc | 37 ++++++++++--------- 3 files changed, 34 insertions(+), 32 deletions(-) diff --git a/include/lsst/meas/algorithms/CoaddPsf.h b/include/lsst/meas/algorithms/CoaddPsf.h index 678b71519..799282189 100644 --- a/include/lsst/meas/algorithms/CoaddPsf.h +++ b/include/lsst/meas/algorithms/CoaddPsf.h @@ -72,7 +72,8 @@ class CoaddPsf : public afw::table::io::PersistableFacade, public Imag * @param[in] warpingKernelName Name of warping kernel * @param[in] cacheSize Warping kernel cache size */ - explicit CoaddPsf(afw::table::ExposureCatalog const& catalog, afw::geom::SkyWcs const& coaddWcs, + explicit CoaddPsf(afw::table::ExposureCatalog const& catalog, + std::shared_ptr coaddWcs, std::string const& weightFieldName = "weight", std::string const& warpingKernelName = "lanczos3", int cacheSize = 10000); @@ -90,12 +91,12 @@ class CoaddPsf : public afw::table::io::PersistableFacade, public Imag * @param[in] weightFieldName Field name that contains the weight of the exposure in the coadd; * defaults to "weight". */ - CoaddPsf(afw::table::ExposureCatalog const& catalog, afw::geom::SkyWcs const& coaddWcs, + CoaddPsf(afw::table::ExposureCatalog const& catalog, std::shared_ptr coaddWcs, CoaddPsfControl const& ctrl, std::string const& weightFieldName = "weight") : CoaddPsf(catalog, coaddWcs, weightFieldName, ctrl.warpingKernelName, ctrl.cacheSize) {} explicit CoaddPsf(afw::table::ExposureCatalog const& catalog, ///< Unpersisted catalog - afw::geom::SkyWcs const& coaddWcs, ///< WCS for the coadd + std::shared_ptr coaddWcs, ///< WCS for the coadd geom::Point2D const& averagePosition, ///< Default position for accessors std::string const& warpingKernelName = "lanczos3", ///< Warping kernel name int cacheSize = 10000 ///< Kernel cache size @@ -115,7 +116,7 @@ class CoaddPsf : public afw::table::io::PersistableFacade, public Imag geom::Point2D getAveragePosition() const override { return _averagePosition; } /// Return the Wcs of the coadd (defines the coordinate system of the Psf). - afw::geom::SkyWcs getCoaddWcs() { return _coaddWcs; } + std::shared_ptr getCoaddWcs() const { return _coaddWcs; } /// Return the number of component Psfs in this CoaddPsf int getComponentCount() const; @@ -127,7 +128,7 @@ class CoaddPsf : public afw::table::io::PersistableFacade, public Imag * @returns Corresponding Psf. * @throws RangeError Index of component is out of range. */ - std::shared_ptr getPsf(int index); + std::shared_ptr getPsf(int index) const; /** * Get the Wcs of the component image at index. @@ -136,7 +137,7 @@ class CoaddPsf : public afw::table::io::PersistableFacade, public Imag * @returns Corresponding Wcs. * @throws RangeError Index of component is out of range. */ - afw::geom::SkyWcs getWcs(int index); + std::shared_ptr getWcs(int index) const; /** * Get the weight of the component image at index. @@ -145,7 +146,7 @@ class CoaddPsf : public afw::table::io::PersistableFacade, public Imag * @returns Corresponding weight. * @throws RangeError Index of component is out of range. */ - double getWeight(int index); + double getWeight(int index) const; /** * Get the exposure ID of the component image at index. @@ -154,7 +155,7 @@ class CoaddPsf : public afw::table::io::PersistableFacade, public Imag * @returns Corresponding exposure ID. * @throws RangeError Index of component is out of range. */ - afw::table::RecordId getId(int index); + afw::table::RecordId getId(int index) const; /** * Get the bounding box (in component image Pixel coordinates) of the component image at index. @@ -163,7 +164,7 @@ class CoaddPsf : public afw::table::io::PersistableFacade, public Imag * @returns Corresponding bounding box. * @throws RangeError Index of component is out of range. */ - geom::Box2I getBBox(int index); + geom::Box2I getBBox(int index) const; /** * Get the validPolygon (in component image Pixel coordinates) of the component image at index. @@ -172,7 +173,7 @@ class CoaddPsf : public afw::table::io::PersistableFacade, public Imag * @returns Corresponding validPolygon. * @throws RangeError Index of component is out of range. */ - std::shared_ptr getValidPolygon(int index); + std::shared_ptr getValidPolygon(int index) const; /** * @brief Return true if the CoaddPsf persistable (always true). @@ -206,7 +207,7 @@ class CoaddPsf : public afw::table::io::PersistableFacade, public Imag private: afw::table::ExposureCatalog _catalog; - afw::geom::SkyWcs _coaddWcs; + std::shared_ptr _coaddWcs; afw::table::Key _weightKey; geom::Point2D _averagePosition; std::string _warpingKernelName; // could be removed if we could get this from _warpingControl (#2949) diff --git a/python/lsst/meas/algorithms/coaddPsf/coaddPsf.cc b/python/lsst/meas/algorithms/coaddPsf/coaddPsf.cc index 282c8010d..93d7029f6 100644 --- a/python/lsst/meas/algorithms/coaddPsf/coaddPsf.cc +++ b/python/lsst/meas/algorithms/coaddPsf/coaddPsf.cc @@ -49,15 +49,15 @@ void declareCoaddPsf(lsst::cpputils::python::WrapperCollection &wrappers) { auto clsCoaddPsf = wrappers.wrapType(PyCoaddPsf(wrappers.module, "CoaddPsf"), [](auto &mod, auto &cls) { /* Constructors */ - cls.def(py::init, std::string const &, std::string const &, int>(), "catalog"_a, "coaddWcs"_a, "weightFieldName"_a = "weight", "warpingKernelName"_a = "lanczos3", "cacheSize"_a = 10000); - cls.def(py::init, geom::Point2D const &, std::string const &, int>(), "catalog"_a, "coaddWcs"_a, "averagePosition"_a, "warpingKernelName"_a = "lanczos3", "cacheSize"_a = 10000); - cls.def(py::init, CoaddPsfControl const &, std::string const &>(), "catalog"_a, "coaddWcs"_a, "ctrl"_a, "weightFieldName"_a = "weight"); diff --git a/src/CoaddPsf.cc b/src/CoaddPsf.cc index b4b332163..87f7e6a7e 100644 --- a/src/CoaddPsf.cc +++ b/src/CoaddPsf.cc @@ -139,7 +139,8 @@ geom::Point2D computeAveragePosition(afw::table::ExposureCatalog const &catalog, } // namespace -CoaddPsf::CoaddPsf(afw::table::ExposureCatalog const &catalog, afw::geom::SkyWcs const &coaddWcs, +CoaddPsf::CoaddPsf(afw::table::ExposureCatalog const &catalog, + std::shared_ptr coaddWcs, std::string const &weightFieldName, std::string const &warpingKernelName, int cacheSize) : _coaddWcs(coaddWcs), _warpingKernelName(warpingKernelName), @@ -165,10 +166,11 @@ CoaddPsf::CoaddPsf(afw::table::ExposureCatalog const &catalog, afw::geom::SkyWcs record->assign(*i, mapper); _catalog.push_back(record); } - _averagePosition = computeAveragePosition(_catalog, _coaddWcs, _weightKey); + _averagePosition = computeAveragePosition(_catalog, *_coaddWcs, _weightKey); } -CoaddPsf::CoaddPsf(afw::table::ExposureCatalog const &catalog, afw::geom::SkyWcs const &coaddWcs, +CoaddPsf::CoaddPsf(afw::table::ExposureCatalog const &catalog, + std::shared_ptr coaddWcs, geom::Point2D const &averagePosition, std::string const &warpingKernelName, int cacheSize) : _catalog(catalog), _coaddWcs(coaddWcs), @@ -227,7 +229,7 @@ void addToImage(std::shared_ptr> image, } // anonymous geom::Box2I CoaddPsf::doComputeBBox(geom::Point2D const &ccdXY, afw::image::Color const &color) const { - afw::table::ExposureCatalog subcat = _catalog.subsetContaining(ccdXY, _coaddWcs, true); + afw::table::ExposureCatalog subcat = _catalog.subsetContaining(ccdXY, *_coaddWcs, true); if (subcat.empty()) { throw LSST_EXCEPT( lsst::afw::detection::InvalidPsfError, @@ -238,7 +240,7 @@ geom::Box2I CoaddPsf::doComputeBBox(geom::Point2D const &ccdXY, afw::image::Colo geom::Box2I ret; for (auto const &exposureRecord : subcat) { // compute transform from exposure pixels to coadd pixels - auto exposureToCoadd = afw::geom::makeWcsPairTransform(*exposureRecord.getWcs(), _coaddWcs); + auto exposureToCoadd = afw::geom::makeWcsPairTransform(*exposureRecord.getWcs(), *_coaddWcs); WarpedPsf warpedPsf = WarpedPsf(exposureRecord.getPsf(), exposureToCoadd, _warpingControl); geom::Box2I componentBBox = warpedPsf.computeBBox(ccdXY, color); ret.include(componentBBox); @@ -250,7 +252,7 @@ geom::Box2I CoaddPsf::doComputeBBox(geom::Point2D const &ccdXY, afw::image::Colo std::shared_ptr CoaddPsf::doComputeKernelImage(geom::Point2D const &ccdXY, afw::image::Color const &color) const { // Get the subset of exposures which contain our coordinate within their validPolygons. - afw::table::ExposureCatalog subcat = _catalog.subsetContaining(ccdXY, _coaddWcs, true); + afw::table::ExposureCatalog subcat = _catalog.subsetContaining(ccdXY, *_coaddWcs, true); if (subcat.empty()) { throw LSST_EXCEPT( lsst::afw::detection::InvalidPsfError, @@ -267,7 +269,7 @@ CoaddPsf::doComputeKernelImage(geom::Point2D const &ccdXY, afw::image::Color con for (auto const &exposureRecord : subcat) { // compute transform from exposure pixels to coadd pixels - auto exposureToCoadd = afw::geom::makeWcsPairTransform(*exposureRecord.getWcs(), _coaddWcs); + auto exposureToCoadd = afw::geom::makeWcsPairTransform(*exposureRecord.getWcs(), *_coaddWcs); std::shared_ptr> componentImg; try { WarpedPsf warpedPsf = WarpedPsf(exposureRecord.getPsf(), exposureToCoadd, _warpingControl); @@ -295,42 +297,42 @@ CoaddPsf::doComputeKernelImage(geom::Point2D const &ccdXY, afw::image::Color con int CoaddPsf::getComponentCount() const { return _catalog.size(); } -std::shared_ptr CoaddPsf::getPsf(int index) { +std::shared_ptr CoaddPsf::getPsf(int index) const { if (index < 0 || index >= getComponentCount()) { throw LSST_EXCEPT(pex::exceptions::RangeError, "index of CoaddPsf component out of range"); } return _catalog[index].getPsf(); } -afw::geom::SkyWcs CoaddPsf::getWcs(int index) { +std::shared_ptr CoaddPsf::getWcs(int index) const { if (index < 0 || index >= getComponentCount()) { throw LSST_EXCEPT(pex::exceptions::RangeError, "index of CoaddPsf component out of range"); } - return *_catalog[index].getWcs(); + return _catalog[index].getWcs(); } -std::shared_ptr CoaddPsf::getValidPolygon(int index) { +std::shared_ptr CoaddPsf::getValidPolygon(int index) const { if (index < 0 || index >= getComponentCount()) { throw LSST_EXCEPT(pex::exceptions::RangeError, "index of CoaddPsf component out of range"); } return _catalog[index].getValidPolygon(); } -double CoaddPsf::getWeight(int index) { +double CoaddPsf::getWeight(int index) const { if (index < 0 || index >= getComponentCount()) { throw LSST_EXCEPT(pex::exceptions::RangeError, "index of CoaddPsf component out of range"); } return _catalog[index].get(_weightKey); } -afw::table::RecordId CoaddPsf::getId(int index) { +afw::table::RecordId CoaddPsf::getId(int index) const { if (index < 0 || index >= getComponentCount()) { throw LSST_EXCEPT(pex::exceptions::RangeError, "index of CoaddPsf component out of range"); } return _catalog[index].getId(); } -geom::Box2I CoaddPsf::getBBox(int index) { +geom::Box2I CoaddPsf::getBBox(int index) const { if (index < 0 || index >= getComponentCount()) { throw LSST_EXCEPT(pex::exceptions::RangeError, "index of CoaddPsf component out of range"); } @@ -388,7 +390,7 @@ class CoaddPsf::Factory : public afw::table::io::PersistableFactory { afw::table::BaseRecord const &record1 = catalogs.front().front(); return std::shared_ptr( new CoaddPsf(afw::table::ExposureCatalog::readFromArchive(archive, catalogs.back()), - *archive.get(record1.get(keys1.coaddWcs)), + archive.get(record1.get(keys1.coaddWcs)), record1.get(keys1.averagePosition), record1.get(keys1.warpingKernelName), record1.get(keys1.cacheSize))); } @@ -413,7 +415,7 @@ class CoaddPsf::Factory : public afw::table::io::PersistableFactory { } catch (pex::exceptions::NotFoundError &) { } auto averagePos = computeAveragePosition(internalCat, *coaddWcs, weightKey); - return std::shared_ptr(new CoaddPsf(internalCat, *coaddWcs, averagePos)); + return std::shared_ptr(new CoaddPsf(internalCat, coaddWcs, averagePos)); } Factory(std::string const &name) : afw::table::io::PersistableFactory(name) {} @@ -435,8 +437,7 @@ void CoaddPsf::write(OutputArchiveHandle &handle) const { CoaddPsfPersistenceHelper const &keys1 = CoaddPsfPersistenceHelper::get(); afw::table::BaseCatalog cat1 = handle.makeCatalog(keys1.schema); std::shared_ptr record1 = cat1.addNew(); - auto coaddWcsPtr = std::make_shared(_coaddWcs); - record1->set(keys1.coaddWcs, handle.put(coaddWcsPtr)); + record1->set(keys1.coaddWcs, handle.put(_coaddWcs)); record1->set(keys1.cacheSize, _warpingControl->getCacheSize()); record1->set(keys1.averagePosition, _averagePosition); record1->set(keys1.warpingKernelName, _warpingKernelName); From 95b3847949b35783425245a6d99a4f57db387248 Mon Sep 17 00:00:00 2001 From: Jim Bosch Date: Tue, 16 Jun 2026 11:51:45 -0400 Subject: [PATCH 2/2] Add accessors for input-image tract and patch on CoaddPsf. In the DIA template_detector, we already store the IDs of the tract and patch of each template coadd in the CoaddPsf, but there is no way to access these. --- include/lsst/meas/algorithms/CoaddPsf.h | 16 +++++++++ .../lsst/meas/algorithms/coaddPsf/coaddPsf.cc | 2 ++ src/CoaddPsf.cc | 33 ++++++++++++++++++- 3 files changed, 50 insertions(+), 1 deletion(-) diff --git a/include/lsst/meas/algorithms/CoaddPsf.h b/include/lsst/meas/algorithms/CoaddPsf.h index 799282189..a82f6fd3b 100644 --- a/include/lsst/meas/algorithms/CoaddPsf.h +++ b/include/lsst/meas/algorithms/CoaddPsf.h @@ -175,6 +175,20 @@ class CoaddPsf : public afw::table::io::PersistableFacade, public Imag */ std::shared_ptr getValidPolygon(int index) const; + /** + * Get the tract ID of the component image at the given index. + * + * @throws lsst::pex::exceptions::NotFoundError There is no tract column. + */ + int getTract(int index) const; + + /** + * Get the patch ID of the component image at the given index. + * + * @throws lsst::pex::exceptions::NotFoundError There is no patch column. + */ + int getPatch(int index) const; + /** * @brief Return true if the CoaddPsf persistable (always true). * @@ -209,6 +223,8 @@ class CoaddPsf : public afw::table::io::PersistableFacade, public Imag afw::table::ExposureCatalog _catalog; std::shared_ptr _coaddWcs; afw::table::Key _weightKey; + afw::table::Key _tractKey; + afw::table::Key _patchKey; geom::Point2D _averagePosition; std::string _warpingKernelName; // could be removed if we could get this from _warpingControl (#2949) std::shared_ptr _warpingControl; diff --git a/python/lsst/meas/algorithms/coaddPsf/coaddPsf.cc b/python/lsst/meas/algorithms/coaddPsf/coaddPsf.cc index 93d7029f6..79e7667f3 100644 --- a/python/lsst/meas/algorithms/coaddPsf/coaddPsf.cc +++ b/python/lsst/meas/algorithms/coaddPsf/coaddPsf.cc @@ -72,6 +72,8 @@ void declareCoaddPsf(lsst::cpputils::python::WrapperCollection &wrappers) { cls.def("getId", &CoaddPsf::getId); cls.def("getBBox", &CoaddPsf::getBBox); cls.def("getValidPolygon", &CoaddPsf::getValidPolygon); + cls.def("getTract", &CoaddPsf::getTract); + cls.def("getPatch", &CoaddPsf::getPatch); cls.def("isPersistable", &CoaddPsf::isPersistable); }); diff --git a/src/CoaddPsf.cc b/src/CoaddPsf.cc index 87f7e6a7e..652d0839e 100644 --- a/src/CoaddPsf.cc +++ b/src/CoaddPsf.cc @@ -175,9 +175,20 @@ CoaddPsf::CoaddPsf(afw::table::ExposureCatalog const &catalog, : _catalog(catalog), _coaddWcs(coaddWcs), _weightKey(_catalog.getSchema()["weight"]), + _tractKey(), + _patchKey(), _averagePosition(averagePosition), _warpingKernelName(warpingKernelName), - _warpingControl(new afw::math::WarpingControl(warpingKernelName, "", cacheSize)) {} + _warpingControl(new afw::math::WarpingControl(warpingKernelName, "", cacheSize)) { + try { + _tractKey = _catalog.getSchema()["tract"]; + } catch (pex::exceptions::NotFoundError &) { + } + try { + _patchKey = _catalog.getSchema()["patch"]; + } catch (pex::exceptions::NotFoundError &) { + } +} std::shared_ptr CoaddPsf::clone() const { return std::make_shared(*this); } @@ -339,6 +350,26 @@ geom::Box2I CoaddPsf::getBBox(int index) const { return _catalog[index].getBBox(); } +int CoaddPsf::getTract(int index) const { + if (index < 0 || index >= getComponentCount()) { + throw LSST_EXCEPT(pex::exceptions::RangeError, "index of CoaddPsf component out of range"); + } + if (!_tractKey.isValid()) { + throw LSST_EXCEPT(pex::exceptions::NotFoundError, "CoaddPsf has no tract column"); + } + return _catalog[index][_tractKey]; +} + +int CoaddPsf::getPatch(int index) const { + if (index < 0 || index >= getComponentCount()) { + throw LSST_EXCEPT(pex::exceptions::RangeError, "index of CoaddPsf component out of range"); + } + if (!_patchKey.isValid()) { + throw LSST_EXCEPT(pex::exceptions::NotFoundError, "CoaddPsf has no patch column"); + } + return _catalog[index][_patchKey]; +} + // ---------- Persistence ----------------------------------------------------------------------------------- // For persistence of CoaddPsf, we have two catalogs: the first has just one record, and contains