From 2200795f30bb4bb1951ec927e4c00dfe37a965f4 Mon Sep 17 00:00:00 2001 From: Jeremy Kubica <104161096+jeremykubica@users.noreply.github.com> Date: Tue, 26 Sep 2023 10:44:23 -0400 Subject: [PATCH 1/2] Add return policies --- src/kbmod/fake_data_creator.py | 6 ------ src/kbmod/search/image_stack.cpp | 4 +++- src/kbmod/search/layered_image.cpp | 16 ++++++++++++---- tests/test_image_stack.py | 3 --- tests/test_layered_image.py | 30 +++++++++--------------------- tests/test_masking.py | 20 -------------------- tests/test_search.py | 6 ------ tests/test_stamp_parity.py | 1 - 8 files changed, 24 insertions(+), 62 deletions(-) diff --git a/src/kbmod/fake_data_creator.py b/src/kbmod/fake_data_creator.py index 9f2cb16ea..29e7c2bb0 100644 --- a/src/kbmod/fake_data_creator.py +++ b/src/kbmod/fake_data_creator.py @@ -47,11 +47,6 @@ def add_fake_object(img, x, y, flux, psf=None): for j in range(dim): sci.add_pixel_interp(initial_x + i, initial_y + j, flux * psf.get_value(i, j)) - # The python/C++ interface requires us to explicitly re-set the science - # image in a LayeredImage. - if sci is not img: - img.set_science(sci) - class FakeDataSet: """This class creates fake data sets for testing and demo notebooks.""" @@ -147,7 +142,6 @@ def insert_object(self, trj): # explicitly because of how pybind handles references. current = self.stack.get_single_image(i) add_fake_object(current, px, py, trj.flux, current.get_psf()) - self.stack.set_single_image(i, current) # Save the trajectory into the internal list. self.trajectories.append(trj) diff --git a/src/kbmod/search/image_stack.cpp b/src/kbmod/search/image_stack.cpp index 6f20219b7..893dd20ae 100644 --- a/src/kbmod/search/image_stack.cpp +++ b/src/kbmod/search/image_stack.cpp @@ -138,7 +138,9 @@ namespace search { .def(py::init, std::vector>()) .def(py::init>()) .def("get_images", &is::get_images, pydocs::DOC_ImageStack_get_images) - .def("get_single_image", &is::get_single_image, pydocs::DOC_ImageStack_get_single_image) + .def("get_single_image", &is::get_single_image, + py::return_value_policy::reference_internal, + pydocs::DOC_ImageStack_get_single_image) .def("set_single_image", &is::set_single_image, pydocs::DOC_ImageStack_set_single_image) .def("get_times", &is::get_times, pydocs::DOC_ImageStack_get_times) .def("set_times", &is::set_times, pydocs::DOC_ImageStack_set_times ) diff --git a/src/kbmod/search/layered_image.cpp b/src/kbmod/search/layered_image.cpp index f741957e8..f536d9780 100644 --- a/src/kbmod/search/layered_image.cpp +++ b/src/kbmod/search/layered_image.cpp @@ -235,14 +235,22 @@ namespace search { .def(py::init()) .def(py::init()) .def("set_psf", &li::set_psf, pydocs::DOC_LayeredImage_set_psf) - .def("get_psf", &li::get_psf, pydocs::DOC_LayeredImage_get_psf) + .def("get_psf", &li::get_psf, + py::return_value_policy::reference_internal, + pydocs::DOC_LayeredImage_get_psf) .def("apply_mask_flags", &li::apply_mask_flags, pydocs::DOC_LayeredImage_apply_mask_flags) .def("apply_mask_threshold", &li::apply_mask_threshold, pydocs::DOC_LayeredImage_apply_mask_threshold) .def("sub_template", &li::subtract_template, pydocs::DOC_LayeredImage_sub_template) .def("save_layers", &li::save_layers, pydocs::DOC_LayeredImage_save_layers) - .def("get_science", &li::get_science, pydocs::DOC_LayeredImage_get_science) - .def("get_mask", &li::get_mask, pydocs::DOC_LayeredImage_get_mask) - .def("get_variance", &li::get_variance, pydocs::DOC_LayeredImage_get_variance) + .def("get_science", &li::get_science, + py::return_value_policy::reference_internal, + pydocs::DOC_LayeredImage_get_science) + .def("get_mask", &li::get_mask, + py::return_value_policy::reference_internal, + pydocs::DOC_LayeredImage_get_mask) + .def("get_variance", &li::get_variance, + py::return_value_policy::reference_internal, + pydocs::DOC_LayeredImage_get_variance) .def("set_science", &li::set_science, pydocs::DOC_LayeredImage_set_science) .def("set_mask", &li::set_mask, pydocs::DOC_LayeredImage_set_mask) .def("set_variance", &li::set_variance, pydocs::DOC_LayeredImage_set_variance) diff --git a/tests/test_image_stack.py b/tests/test_image_stack.py index 66b0b4f26..52e4327ab 100644 --- a/tests/test_image_stack.py +++ b/tests/test_image_stack.py @@ -26,7 +26,6 @@ def setUp(self): # Include one masked pixel per time step at (10, 10 + i). mask = self.images[i].get_mask() mask.set_pixel(10, 10 + i, 1) - self.images[i].set_mask(mask) self.im_stack = ImageStack(self.images) @@ -133,8 +132,6 @@ def test_create_global_mask_reset(self): img = self.im_stack.get_single_image(i) mask = img.get_mask() mask.set_pixel(10, 10 + i, 0) - img.set_mask(mask) - self.im_stack.set_single_image(i, img) # Reapply the mask and check that nothing is masked. # Note the science pixels will still be masked from the previous application. diff --git a/tests/test_layered_image.py b/tests/test_layered_image.py index 8dbcd702c..deb1d4134 100644 --- a/tests/test_layered_image.py +++ b/tests/test_layered_image.py @@ -169,7 +169,6 @@ def test_apply_mask(self): mask.set_pixel(10, 11, 1) mask.set_pixel(10, 12, 2) mask.set_pixel(10, 13, 3) - self.image.set_mask(mask) # Apply the mask flags to only (10, 11) and (10, 13) self.image.apply_mask_flags(1, []) @@ -187,7 +186,6 @@ def test_apply_mask_exceptions(self): mask.set_pixel(10, 11, 1) mask.set_pixel(10, 12, 2) mask.set_pixel(10, 13, 3) - self.image.set_mask(mask) # Apply the mask flags to only (10, 11). self.image.apply_mask_flags(1, [1]) @@ -205,7 +203,6 @@ def test_grow_mask(self): mask.set_pixel(10, 11, 1) mask.set_pixel(10, 12, 1) mask.set_pixel(10, 13, 1) - self.image.set_mask(mask) self.image.apply_mask_flags(1, []) self.image.grow_mask(1) @@ -224,7 +221,6 @@ def test_grow_mask_mult(self): mask = self.image.get_mask() mask.set_pixel(10, 11, 1) mask.set_pixel(10, 12, 1) - self.image.set_mask(mask) self.image.apply_mask_flags(1, []) self.image.grow_mask(3) @@ -250,8 +246,6 @@ def test_psi_and_phi_image(self): sci.set_pixel(x, y, float(x)) var.set_pixel(x, y, float(y + 1)) var.set_pixel(3, 1, KB_NO_DATA) - img.set_science(sci) - img.set_variance(var) # Generate and check psi and phi images. psi = img.generate_psi_image() @@ -272,26 +266,21 @@ def test_psi_and_phi_image(self): self.assertAlmostEqual(phi.get_pixel(x, y), 1.0 / float(y + 1)) def test_subtract_template(self): - old_science = self.image.get_science() - - # Mask out a few points and reset (needed because of how pybind handles - # pass by reference). - old_science.set_pixel(5, 6, KB_NO_DATA) - old_science.set_pixel(10, 7, KB_NO_DATA) - old_science.set_pixel(10, 21, KB_NO_DATA) - self.image.set_science(old_science) + sci = self.image.get_science() + sci.set_pixel(10, 7, KB_NO_DATA) + sci.set_pixel(10, 21, KB_NO_DATA) + old_sci = RawImage(sci) # Make a copy. template = RawImage(self.image.get_width(), self.image.get_height()) template.set_all(0.0) - for h in range(old_science.get_height()): + for h in range(sci.get_height()): template.set_pixel(10, h, 0.01 * h) self.image.sub_template(template) - new_science = self.image.get_science() - for x in range(old_science.get_width()): - for y in range(old_science.get_height()): - val1 = old_science.get_pixel(x, y) - val2 = new_science.get_pixel(x, y) + for x in range(sci.get_width()): + for y in range(sci.get_height()): + val1 = old_sci.get_pixel(x, y) + val2 = sci.get_pixel(x, y) if x == 10 and y != 7 and y != 21: self.assertAlmostEqual(val2, val1 - 0.01 * y, delta=1e-6) else: @@ -316,7 +305,6 @@ def test_read_write_files(self): mask1 = im1.get_mask() mask1.set_pixel(3, 5, 1.0) mask1.set_pixel(5, 3, 1.0) - im1.set_mask(mask1) # Save the test data. im1.save_layers(dir_name + "/") diff --git a/tests/test_masking.py b/tests/test_masking.py index b7ebc248f..49e22760e 100644 --- a/tests/test_masking.py +++ b/tests/test_masking.py @@ -61,10 +61,6 @@ def test_threshold_masker(self): sci.set_pixel(2 + i, 8, 501.0) sci.set_pixel(1 + i, 9, 499.0) - # We need to reset the images because of how pybind handles pass by reference. - img.set_science(sci) - self.stack.set_single_image(i, img) - # With a threshold of 500 one pixel per image should be masked. mask = ThresholdMask(500) self.stack = mask.apply_mask(self.stack) @@ -85,10 +81,6 @@ def test_per_image_dictionary_mask(self): for x in range(self.dim_x): msk.set_pixel(x, 3, 2**x) - # We need to reset the images because of how pybind handles pass by reference. - img.set_mask(msk) - self.stack.set_single_image(i, img) - # Mask with two keys. mask = DictionaryMasker(self.mask_bits_dict, ["BAD", "EDGE"]) self.stack = mask.apply_mask(self.stack) @@ -121,10 +113,6 @@ def test_mask_grow(self): for x in range(self.dim_x): msk.set_pixel(2 + i, 8, 1) - # We need to reset the images because of how pybind handles pass by reference. - img.set_mask(msk) - self.stack.set_single_image(i, img) - # Apply the bit vector based mask and check that one pixel per image is masked. self.stack = BitVectorMasker(1, []).apply_mask(self.stack) for i in range(self.img_count): @@ -156,10 +144,6 @@ def test_global_mask(self): if i == 0: msk.set_pixel(5, 5, 4) - # We need to reset the images because of how pybind handles pass by reference. - img.set_mask(msk) - self.stack.set_single_image(i, img) - mask = GlobalDictionaryMasker(self.mask_bits_dict, ["CR", "INTRP"], 2) self.stack = mask.apply_mask(self.stack) for i in range(self.img_count): @@ -228,10 +212,6 @@ def test_apply_masks(self): msk.set_pixel(6, 5, 8) bad_pixels.append((i, 6, 5)) - # We need to reset the images because of how pybind handles pass by reference. - img.set_science(sci) - img.set_mask(msk) - self.stack.set_single_image(i, img) bad_set = set(bad_pixels) # Do the actual masking. diff --git a/tests/test_search.py b/tests/test_search.py index e06a76d8b..cf0c906b8 100644 --- a/tests/test_search.py +++ b/tests/test_search.py @@ -69,7 +69,6 @@ def setUp(self): if i % 2 == 0: mask = im.get_mask() mask.set_pixel(self.masked_x, self.masked_y, 1) - im.set_mask(mask) im.apply_mask_flags(1, []) self.imlist.append(im) @@ -103,7 +102,6 @@ def test_psiphi(self): mask = image2.get_mask() mask.set_pixel(4, 9, 1) - image2.set_mask(mask) image2.apply_mask_flags(1, []) # Create a stack from the two objects. @@ -477,7 +475,6 @@ def test_coadd_cpu_simple(self): sci = im.get_science() for x in range(3): sci.set_pixel(x, 1, i + 1) - im.set_science(sci) # Mask out the row's first pixel twice and second pixel once. mask = im.get_mask() @@ -486,7 +483,6 @@ def test_coadd_cpu_simple(self): mask.set_pixel(1, 1, 1) if i == 1: mask.set_pixel(0, 1, 1) - im.set_mask(mask) im.apply_mask_flags(1, []) imlist.append(im) @@ -539,7 +535,6 @@ def test_coadd_gpu_simple(self): sci = im.get_science() for x in range(3): sci.set_pixel(x, 1, i + 1) - im.set_science(sci) # Mask out the row's first pixel twice and second pixel once. mask = im.get_mask() @@ -548,7 +543,6 @@ def test_coadd_gpu_simple(self): mask.set_pixel(1, 1, 1) if i == 1: mask.set_pixel(0, 1, 1) - im.set_mask(mask) im.apply_mask_flags(1, []) imlist.append(im) diff --git a/tests/test_stamp_parity.py b/tests/test_stamp_parity.py index e1286ec28..0a527d923 100644 --- a/tests/test_stamp_parity.py +++ b/tests/test_stamp_parity.py @@ -72,7 +72,6 @@ def setUp(self): if i % 2 == 0: mask = im.get_mask() mask.set_pixel(self.masked_x, self.masked_y, 1) - im.set_mask(mask) im.apply_mask_flags(1, []) self.imlist.append(im) From 3849a7e41d9588604af164483de4895031529858 Mon Sep 17 00:00:00 2001 From: Jeremy Kubica <104161096+jeremykubica@users.noreply.github.com> Date: Wed, 27 Sep 2023 11:06:21 -0400 Subject: [PATCH 2/2] Add return type for get_imagestack --- src/kbmod/run_search.py | 4 ++-- src/kbmod/search/stack_search.cpp | 4 +++- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/src/kbmod/run_search.py b/src/kbmod/run_search.py index da5824137..f0c05be56 100644 --- a/src/kbmod/run_search.py +++ b/src/kbmod/run_search.py @@ -131,13 +131,13 @@ def do_gpu_search(self, search, img_info, suggested_angle, post_process): if self.config["x_pixel_bounds"] and len(self.config["x_pixel_bounds"]) == 2: search.set_start_bounds_x(self.config["x_pixel_bounds"][0], self.config["x_pixel_bounds"][1]) elif self.config["x_pixel_buffer"] and self.config["x_pixel_buffer"] > 0: - width = search.get_ImageStack().get_width() + width = search.get_imagestack().get_width() search.set_start_bounds_x(-self.config["x_pixel_buffer"], width + self.config["x_pixel_buffer"]) if self.config["y_pixel_bounds"] and len(self.config["y_pixel_bounds"]) == 2: search.set_start_bounds_y(self.config["y_pixel_bounds"][0], self.config["y_pixel_bounds"][1]) elif self.config["y_pixel_buffer"] and self.config["y_pixel_buffer"] > 0: - height = search.get_ImageStack().get_height() + height = search.get_imagestack().get_height() search.set_start_bounds_y(-self.config["y_pixel_buffer"], height + self.config["y_pixel_buffer"]) # If we are using barycentric corrections, compute the parameters and diff --git a/src/kbmod/search/stack_search.cpp b/src/kbmod/search/stack_search.cpp index 71dcdca21..ca2730108 100644 --- a/src/kbmod/search/stack_search.cpp +++ b/src/kbmod/search/stack_search.cpp @@ -625,7 +625,9 @@ namespace search { .def("set_debug", &ks::set_debug, pydocs::DOC_StackSearch_set_debug) .def("filter_min_obs", &ks::filter_results, pydocs::DOC_StackSearch_filter_min_obs) .def("get_num_images", &ks::num_images, pydocs::DOC_StackSearch_get_num_images) - .def("get_imagestack", &ks::get_imagestack, pydocs::DOC_StackSearch_get_imagestack) + .def("get_imagestack", &ks::get_imagestack, + py::return_value_policy::reference_internal, + pydocs::DOC_StackSearch_get_imagestack) // Science Stamp Functions .def("get_stamps", &ks::get_stamps, pydocs::DOC_StackSearch_get_stamps) .def("get_median_stamp", &ks::get_median_stamp, pydocs::DOC_StackSearch_get_median_stamp)