From 13e54c804b1842c25e8224dcc9ca09eab46daabe Mon Sep 17 00:00:00 2001 From: Jeremy Kubica <104161096+jeremykubica@users.noreply.github.com> Date: Fri, 23 Feb 2024 14:15:25 -0500 Subject: [PATCH 1/4] Add basic validation to the PSF object --- src/kbmod/search/psf.cpp | 7 ++++++- tests/test_psf.py | 18 +++++++++++++++++- 2 files changed, 23 insertions(+), 2 deletions(-) diff --git a/src/kbmod/search/psf.cpp b/src/kbmod/search/psf.cpp index fa2735cbb..ee25f75a3 100644 --- a/src/kbmod/search/psf.cpp +++ b/src/kbmod/search/psf.cpp @@ -87,7 +87,12 @@ PSF& PSF::operator=(PSF&& other) { void PSF::calc_sum() { sum = 0.0; - for (auto& i : kernel) sum += i; + for (auto& i : kernel) { + if (std::isnan(i) || std::isinf(i)) { + throw std::runtime_error("Invalid value in PSF kernel (NaN or inf)"); + } + sum += i; + } } void PSF::square_psf() { diff --git a/tests/test_psf.py b/tests/test_psf.py index a7eba0ea9..2d06117be 100644 --- a/tests/test_psf.py +++ b/tests/test_psf.py @@ -1,3 +1,5 @@ +import math +import numpy as np import unittest from kbmod.search import PSF @@ -19,10 +21,24 @@ def test_make_noop(self): self.assertEqual(len(kernel0), 1) self.assertEqual(kernel0[0], 1.0) - def test_make_invalud(self): + def test_make_invalid(self): # Raise an error if creating a PSF with a negative stdev. self.assertRaises(RuntimeError, PSF, -1.0) + def test_make_from_array(self): + arr = np.full((3, 3), 1.0/9.0) + psf_arr = PSF(arr) + self.assertEqual(psf_arr.get_size(), 9) + self.assertEqual(psf_arr.get_dim(), 3) + + # We get an error if we include a NaN. + arr[0][0] = math.nan + self.assertRaises(RuntimeError, PSF, arr) + + # We get an error if we include a inf. + arr[0][0] = math.inf + self.assertRaises(RuntimeError, PSF, arr) + def test_to_string(self): result = self.psf_list[0].__str__() self.assertGreater(len(result), 1) From d43dee872fdc29244e1cc08fc1ea01d8104761e1 Mon Sep 17 00:00:00 2001 From: Jeremy Kubica <104161096+jeremykubica@users.noreply.github.com> Date: Fri, 23 Feb 2024 14:21:30 -0500 Subject: [PATCH 2/4] Fix formatting --- tests/test_psf.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_psf.py b/tests/test_psf.py index 2d06117be..bb37d9157 100644 --- a/tests/test_psf.py +++ b/tests/test_psf.py @@ -26,7 +26,7 @@ def test_make_invalid(self): self.assertRaises(RuntimeError, PSF, -1.0) def test_make_from_array(self): - arr = np.full((3, 3), 1.0/9.0) + arr = np.full((3, 3), 1.0 / 9.0) psf_arr = PSF(arr) self.assertEqual(psf_arr.get_size(), 9) self.assertEqual(psf_arr.get_dim(), 3) From dbdf8d9a102301bbc5d3bca6870ee08a80ff6e76 Mon Sep 17 00:00:00 2001 From: Jeremy Kubica <104161096+jeremykubica@users.noreply.github.com> Date: Fri, 23 Feb 2024 14:52:12 -0500 Subject: [PATCH 3/4] Remove unused attribute Remove an unused attrbiute. --- src/kbmod/search/psf.cpp | 14 ++------------ src/kbmod/search/psf.h | 8 ++++---- src/kbmod/search/pydocs/psf_docs.h | 11 ++++------- 3 files changed, 10 insertions(+), 23 deletions(-) diff --git a/src/kbmod/search/psf.cpp b/src/kbmod/search/psf.cpp index ee25f75a3..ee2c7ee9b 100644 --- a/src/kbmod/search/psf.cpp +++ b/src/kbmod/search/psf.cpp @@ -4,7 +4,6 @@ namespace search { PSF::PSF() : kernel(1, 1.0) { dim = 1; radius = 0; - width = 1e-12; sum = 1.0; } @@ -13,7 +12,7 @@ PSF::PSF(float stdev) { throw std::runtime_error("PSF stdev must be > 0.0."); } - width = stdev; + float width = stdev; float simple_gauss[MAX_KERNEL_RADIUS]; double psf_coverage = 0.0; double norm_factor = stdev * sqrt(2.0); @@ -51,7 +50,6 @@ PSF::PSF(const PSF& other) { kernel = other.kernel; dim = other.dim; radius = other.radius; - width = other.width; sum = other.sum; } @@ -60,18 +58,13 @@ PSF& PSF::operator=(const PSF& other) { kernel = other.kernel; dim = other.dim; radius = other.radius; - width = other.width; sum = other.sum; return *this; } // Move constructor. PSF::PSF(PSF&& other) - : kernel(std::move(other.kernel)), - dim(other.dim), - radius(other.radius), - width(other.width), - sum(other.sum) {} + : kernel(std::move(other.kernel)), dim(other.dim), radius(other.radius), sum(other.sum) {} // Move assignment. PSF& PSF::operator=(PSF&& other) { @@ -79,7 +72,6 @@ PSF& PSF::operator=(PSF&& other) { kernel = std::move(other.kernel); dim = other.dim; radius = other.radius; - width = other.width; sum = other.sum; } return *this; @@ -144,7 +136,6 @@ void PSF::set_array(pybind11::array_t arr) { sum = 0.0; kernel = std::vector(pix, pix + dim * dim); calc_sum(); - width = 0.0; } static void psf_bindings(py::module& m) { @@ -166,7 +157,6 @@ static void psf_bindings(py::module& m) { .def(py::init()) .def("__str__", &psf::print) .def("set_array", &psf::set_array, pydocs::DOC_PSF_set_array) - .def("get_std", &psf::get_std, pydocs::DOC_PSF_get_std) .def("get_sum", &psf::get_sum, pydocs::DOC_PSF_get_sum) .def("get_dim", &psf::get_dim, pydocs::DOC_PSF_get_dim) .def("get_radius", &psf::get_radius, pydocs::DOC_PSF_get_radius) diff --git a/src/kbmod/search/psf.h b/src/kbmod/search/psf.h index 37b7a4d26..0985a654b 100644 --- a/src/kbmod/search/psf.h +++ b/src/kbmod/search/psf.h @@ -28,23 +28,23 @@ class PSF { PSF& operator=(PSF&& other); // Move assignment // Getter functions (inlined) - float get_std() const { return width; } float get_sum() const { return sum; } float get_value(int x, int y) const { return kernel[y * dim + x]; } - int get_dim() const { return dim; } + int get_dim() const { return dim; } // Length of one side of the kernel. int get_radius() const { return radius; } int get_size() const { return kernel.size(); } const std::vector& get_kernel() const { return kernel; }; float* data() { return kernel.data(); } // Computation functions. - void calc_sum(); void square_psf(); std::string print(); private: + // Validates the PSF array and computes the sum. + void calc_sum(); + std::vector kernel; - float width; float sum; int dim; int radius; diff --git a/src/kbmod/search/pydocs/psf_docs.h b/src/kbmod/search/pydocs/psf_docs.h index a58fb07f4..50fbe421d 100644 --- a/src/kbmod/search/pydocs/psf_docs.h +++ b/src/kbmod/search/pydocs/psf_docs.h @@ -23,7 +23,8 @@ static const auto DOC_PSF = R"doc( Raises ------ - Raises a ``RuntimeError`` when given an invalid stdev. + Raises a ``RuntimeError`` when given an invalid stdev or an array + containing invalid entries, such as NaN or infinity. )doc"; static const auto DOC_PSF_set_array = R"doc( @@ -40,20 +41,16 @@ static const auto DOC_PSF_set_array = R"doc( matrix. )doc"; -static const auto DOC_PSF_get_std = R"doc( - "Returns the PSF's standard deviation." - )doc"; - static const auto DOC_PSF_get_sum = R"doc( "Returns the sum of PSFs kernel elements. ")doc"; static const auto DOC_PSF_get_dim = R"doc( - "Returns the PSF kernel dimensions. + "Returns the PSF kernel dimension D where the kernel is a D by D array. ")doc"; static const auto DOC_PSF_get_radius = R"doc( - "Returns the radius of the PSF + "Returns the radius of the PSF. ")doc"; static const auto DOC_PSF_get_size = R"doc( From 3fe611476df27515d223baa7e718c2ce77e562d3 Mon Sep 17 00:00:00 2001 From: Jeremy Kubica <104161096+jeremykubica@users.noreply.github.com> Date: Fri, 23 Feb 2024 14:58:35 -0500 Subject: [PATCH 4/4] Revert width removal --- src/kbmod/search/psf.cpp | 14 ++++++++++++-- src/kbmod/search/psf.h | 2 ++ src/kbmod/search/pydocs/psf_docs.h | 4 ++++ 3 files changed, 18 insertions(+), 2 deletions(-) diff --git a/src/kbmod/search/psf.cpp b/src/kbmod/search/psf.cpp index ee2c7ee9b..ee25f75a3 100644 --- a/src/kbmod/search/psf.cpp +++ b/src/kbmod/search/psf.cpp @@ -4,6 +4,7 @@ namespace search { PSF::PSF() : kernel(1, 1.0) { dim = 1; radius = 0; + width = 1e-12; sum = 1.0; } @@ -12,7 +13,7 @@ PSF::PSF(float stdev) { throw std::runtime_error("PSF stdev must be > 0.0."); } - float width = stdev; + width = stdev; float simple_gauss[MAX_KERNEL_RADIUS]; double psf_coverage = 0.0; double norm_factor = stdev * sqrt(2.0); @@ -50,6 +51,7 @@ PSF::PSF(const PSF& other) { kernel = other.kernel; dim = other.dim; radius = other.radius; + width = other.width; sum = other.sum; } @@ -58,13 +60,18 @@ PSF& PSF::operator=(const PSF& other) { kernel = other.kernel; dim = other.dim; radius = other.radius; + width = other.width; sum = other.sum; return *this; } // Move constructor. PSF::PSF(PSF&& other) - : kernel(std::move(other.kernel)), dim(other.dim), radius(other.radius), sum(other.sum) {} + : kernel(std::move(other.kernel)), + dim(other.dim), + radius(other.radius), + width(other.width), + sum(other.sum) {} // Move assignment. PSF& PSF::operator=(PSF&& other) { @@ -72,6 +79,7 @@ PSF& PSF::operator=(PSF&& other) { kernel = std::move(other.kernel); dim = other.dim; radius = other.radius; + width = other.width; sum = other.sum; } return *this; @@ -136,6 +144,7 @@ void PSF::set_array(pybind11::array_t arr) { sum = 0.0; kernel = std::vector(pix, pix + dim * dim); calc_sum(); + width = 0.0; } static void psf_bindings(py::module& m) { @@ -157,6 +166,7 @@ static void psf_bindings(py::module& m) { .def(py::init()) .def("__str__", &psf::print) .def("set_array", &psf::set_array, pydocs::DOC_PSF_set_array) + .def("get_std", &psf::get_std, pydocs::DOC_PSF_get_std) .def("get_sum", &psf::get_sum, pydocs::DOC_PSF_get_sum) .def("get_dim", &psf::get_dim, pydocs::DOC_PSF_get_dim) .def("get_radius", &psf::get_radius, pydocs::DOC_PSF_get_radius) diff --git a/src/kbmod/search/psf.h b/src/kbmod/search/psf.h index 0985a654b..2dc96a17b 100644 --- a/src/kbmod/search/psf.h +++ b/src/kbmod/search/psf.h @@ -28,6 +28,7 @@ class PSF { PSF& operator=(PSF&& other); // Move assignment // Getter functions (inlined) + float get_std() const { return width; } float get_sum() const { return sum; } float get_value(int x, int y) const { return kernel[y * dim + x]; } int get_dim() const { return dim; } // Length of one side of the kernel. @@ -45,6 +46,7 @@ class PSF { void calc_sum(); std::vector kernel; + float width; float sum; int dim; int radius; diff --git a/src/kbmod/search/pydocs/psf_docs.h b/src/kbmod/search/pydocs/psf_docs.h index 50fbe421d..22e18cabd 100644 --- a/src/kbmod/search/pydocs/psf_docs.h +++ b/src/kbmod/search/pydocs/psf_docs.h @@ -41,6 +41,10 @@ static const auto DOC_PSF_set_array = R"doc( matrix. )doc"; +static const auto DOC_PSF_get_std = R"doc( + "Returns the PSF's standard deviation." + )doc"; + static const auto DOC_PSF_get_sum = R"doc( "Returns the sum of PSFs kernel elements. ")doc";