From f0cd10bc4d9fa9ad05c398832bb0ec32f99d7278 Mon Sep 17 00:00:00 2001 From: "M. Teodoro" Date: Tue, 20 Aug 2024 15:17:39 -0400 Subject: [PATCH 01/20] Allow updating source catalog with tweaked WCS when running ELP. --- romancal/pipeline/exposure_pipeline.py | 2 + romancal/tweakreg/tweakreg_step.py | 129 ++++++++++++++++++------- 2 files changed, 96 insertions(+), 35 deletions(-) diff --git a/romancal/pipeline/exposure_pipeline.py b/romancal/pipeline/exposure_pipeline.py index 0ba0aeafa..fce8c6a85 100644 --- a/romancal/pipeline/exposure_pipeline.py +++ b/romancal/pipeline/exposure_pipeline.py @@ -70,6 +70,8 @@ def process(self, input): # make sure source_catalog returns the updated datamodel self.source_catalog.return_updated_model = True + # make sure we update source catalog coordinates afer running TweakRegStep + self.tweakreg.update_source_catalog_coordinates = True log.info("Starting Roman exposure calibration pipeline ...") if isinstance(input, str): diff --git a/romancal/tweakreg/tweakreg_step.py b/romancal/tweakreg/tweakreg_step.py index 080486d99..bf6b24582 100644 --- a/romancal/tweakreg/tweakreg_step.py +++ b/romancal/tweakreg/tweakreg_step.py @@ -162,41 +162,7 @@ def process(self, input): images.shelve(image_model) return image_model - if hasattr(image_model.meta, "source_detection"): - is_tweakreg_catalog_present = hasattr( - image_model.meta.source_detection, "tweakreg_catalog" - ) - is_tweakreg_catalog_name_present = hasattr( - image_model.meta.source_detection, "tweakreg_catalog_name" - ) - if is_tweakreg_catalog_present: - # read catalog from structured array - catalog = Table( - np.asarray( - image_model.meta.source_detection.tweakreg_catalog - ) - ) - elif is_tweakreg_catalog_name_present: - catalog = self.read_catalog( - image_model.meta.source_detection.tweakreg_catalog_name - ) - else: - images.shelve(image_model, i, modify=False) - raise AttributeError( - "Attribute 'meta.source_detection.tweakreg_catalog' is missing." - "Please either run SourceDetectionStep or provide a" - "custom source catalog." - ) - # remove 4D numpy array from meta.source_detection - if is_tweakreg_catalog_present: - del image_model.meta.source_detection["tweakreg_catalog"] - else: - images.shelve(image_model, i, modify=False) - raise AttributeError( - "Attribute 'meta.source_detection' is missing." - "Please either run SourceDetectionStep or provide a" - "custom source catalog." - ) + catalog = self.get_tweakreg_catalog(images, i, image_model) for axis in ["x", "y"]: if axis not in catalog.colnames: @@ -495,10 +461,103 @@ def process(self, input): del image_model.meta["wcs_fit_results"][k] image_model.meta.wcs = imcat.wcs + + if self.update_source_catalog_coordinates: + self.update_catalog_coordinates( + image_model.meta.source_detection.tweakreg_catalog_name, + image_model.meta.wcs, + ) images.shelve(image_model, i) return images + def get_tweakreg_catalog(self, images, i, image_model): + if hasattr(image_model.meta, "source_detection"): + is_tweakreg_catalog_present = hasattr( + image_model.meta.source_detection, "tweakreg_catalog" + ) + is_tweakreg_catalog_name_present = hasattr( + image_model.meta.source_detection, "tweakreg_catalog_name" + ) + if is_tweakreg_catalog_present: + # read catalog from structured array + catalog = Table( + np.asarray(image_model.meta.source_detection.tweakreg_catalog) + ) + elif is_tweakreg_catalog_name_present: + catalog = self.read_catalog( + image_model.meta.source_detection.tweakreg_catalog_name + ) + else: + images.shelve(image_model, i, modify=False) + raise AttributeError( + "Attribute 'meta.source_detection.tweakreg_catalog' is missing." + "Please either run SourceDetectionStep or provide a" + "custom source catalog." + ) + # remove 4D numpy array from meta.source_detection + if is_tweakreg_catalog_present: + del image_model.meta.source_detection["tweakreg_catalog"] + else: + images.shelve(image_model, i, modify=False) + raise AttributeError( + "Attribute 'meta.source_detection' is missing." + "Please either run SourceDetectionStep or provide a" + "custom source catalog." + ) + + return catalog + + def update_catalog_coordinates(self, tweakreg_catalog_name, tweaked_wcs): + """ + Update the source catalog coordinates using the tweaked WCS. + + Parameters + ---------- + tweakreg_catalog_name : str + The name of the TweakReg catalog file produced by `SourceCatalog`. + tweaked_wcs : `astropy.wcs.WCS` + The tweaked World Coordinate System (WCS) object. + + Returns + ------- + None + """ + # read in cat file + with rdm.open(tweakreg_catalog_name) as source_catalog_model: + # get catalog + catalog = source_catalog_model.source_catalog + + # define mapping between pixel and world coordinates + colname_mapping = { + ("xcentroid", "ycentroid"): ("ra_centroid", "dec_centroid"), + ("x_psf", "y_psf"): ("ra_psf", "dec_psf"), + } + + for k, v in colname_mapping.items(): + # get column names + x_colname, y_colname = k + ra_colname, dec_colname = v + + # calculate new coordinates using tweaked WCS + tweaked_centroid = tweaked_wcs.pixel_to_world( + catalog[x_colname], catalog[y_colname] + ) + + # update catalog coordinates + catalog[ra_colname], catalog[dec_colname] = ( + tweaked_centroid.ra.deg, + tweaked_centroid.dec.deg, + ) + + # save updated catalog (overwrite cat file) + self.save_model( + source_catalog_model, + output_file=source_catalog_model.meta.filename, + suffix="cat", + force=True, + ) + def read_catalog(self, catalog_name): if catalog_name.endswith("asdf"): with rdm.open(catalog_name) as source_catalog_model: From 478cdfc6339c1b496099355749455f0970d8d3b7 Mon Sep 17 00:00:00 2001 From: "M. Teodoro" Date: Tue, 20 Aug 2024 15:27:49 -0400 Subject: [PATCH 02/20] Add changelog entry. --- CHANGES.rst | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/CHANGES.rst b/CHANGES.rst index 14dffba59..8ab0e0926 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -1,7 +1,10 @@ 0.16.2 (unreleased) =================== -- +exposure_pipeline +----------------- +- Update source catalog file with tweaked coordinates. [#1373] + 0.16.1 (2024-08-13) =================== From 06507bd69c8a087e83c0d064a24cfc9141712674 Mon Sep 17 00:00:00 2001 From: "M. Teodoro" Date: Tue, 20 Aug 2024 15:38:25 -0400 Subject: [PATCH 03/20] Fix how attribute is checked. --- romancal/tweakreg/tweakreg_step.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/romancal/tweakreg/tweakreg_step.py b/romancal/tweakreg/tweakreg_step.py index bf6b24582..7f151efd7 100644 --- a/romancal/tweakreg/tweakreg_step.py +++ b/romancal/tweakreg/tweakreg_step.py @@ -462,7 +462,7 @@ def process(self, input): image_model.meta.wcs = imcat.wcs - if self.update_source_catalog_coordinates: + if getattr(self, "update_source_catalog_coordinates", False): self.update_catalog_coordinates( image_model.meta.source_detection.tweakreg_catalog_name, image_model.meta.wcs, From 6d4fed5791a9513feee8f9c738fa9ba78aedbb6b Mon Sep 17 00:00:00 2001 From: "M. Teodoro" Date: Thu, 22 Aug 2024 14:19:16 -0400 Subject: [PATCH 04/20] Add unit test to new method in TweakRegStep. --- romancal/tweakreg/tests/test_tweakreg.py | 124 +++++++++++++++++++++++ 1 file changed, 124 insertions(+) diff --git a/romancal/tweakreg/tests/test_tweakreg.py b/romancal/tweakreg/tests/test_tweakreg.py index 9413b6ad1..3cb778081 100644 --- a/romancal/tweakreg/tests/test_tweakreg.py +++ b/romancal/tweakreg/tests/test_tweakreg.py @@ -3,6 +3,7 @@ import os from io import StringIO from pathlib import Path +import random from typing import Tuple import numpy as np @@ -1185,3 +1186,126 @@ def test_parse_catfile_raises_error_on_invalid_content(tmp_path, catfile_line_co trs._parse_catfile(catfile) assert type(exec_info.value) == ValueError + + +def test_update_source_catalog_coordinates(tmp_path, base_image): + """Test that TweakReg updates the catalog coordinates with the tweaked WCS.""" + + os.chdir(tmp_path) + + img = base_image(shift_1=1000, shift_2=1000) + add_tweakreg_catalog_attribute(tmp_path, img, catalog_filename="img_1") + + tweakreg = trs.TweakRegStep() + + # create SourceCatalogModel + source_catalog_model = setup_source_catalog_model(img) + + # save SourceCatalogModel + tweakreg.save_model( + source_catalog_model, + output_file="img_1.asdf", + suffix="cat", + force=True, + ) + + # update tweakreg catalog name + img.meta.source_detection.tweakreg_catalog_name = "img_1_cat.asdf" + + # run TweakRegStep + res = trs.TweakRegStep.call([img]) + + # tweak the current WCS using TweakRegStep and save the updated cat file + with res: + dm = res.borrow(0) + assert dm.meta.source_detection.tweakreg_catalog_name == "img_1_cat.asdf" + tweakreg.update_catalog_coordinates( + dm.meta.source_detection.tweakreg_catalog_name, dm.meta.wcs + ) + res.shelve(dm, 0) + + # read in saved catalog coords + cat = rdm.open("img_1_cat.asdf") + cat_ra_centroid = cat.source_catalog["ra_centroid"] + cat_dec_centroid = cat.source_catalog["dec_centroid"] + cat_ra_psf = cat.source_catalog["ra_psf"] + cat_dec_psf = cat.source_catalog["dec_psf"] + + # calculate world coords using tweaked WCS + expected_centroid = img.meta.wcs.pixel_to_world( + cat.source_catalog["xcentroid"], cat.source_catalog["ycentroid"] + ) + expected_psf = img.meta.wcs.pixel_to_world( + cat.source_catalog["x_psf"], cat.source_catalog["y_psf"] + ) + + # compare coordinates (make sure tweaked WCS was applied to cat file coords) + np.testing.assert_array_equal(cat_ra_centroid, expected_centroid.ra.value) + np.testing.assert_array_equal(cat_dec_centroid, expected_centroid.dec.value) + np.testing.assert_array_equal(cat_ra_psf, expected_psf.ra.value) + np.testing.assert_array_equal(cat_dec_psf, expected_psf.dec.value) + + +def setup_source_catalog_model(img): + """ + Set up the source catalog model. + + Notes + ----- + This function reads the source catalog from a file, renames columns to match + expected names, adds mock PSF coordinates, applies random shifts to the centroid + and PSF coordinates, and calculates the world coordinates for the centroids. + """ + cat_model = rdm.SourceCatalogModel + source_catalog_model = maker_utils.mk_datamodel(cat_model) + # this will be the output filename + source_catalog_model.meta.filename = "img_1.asdf" + + # read in the mock table + source_catalog = Table.read("img_1", format="ascii.ecsv") + # rename columns to match expected column names + source_catalog.rename_columns(["x", "y"], ["xcentroid", "ycentroid"]) + # add mock PSF coordinates + source_catalog["x_psf"] = source_catalog["xcentroid"] + source_catalog["y_psf"] = source_catalog["ycentroid"] + + # add random fraction of a pixel shifts to the centroid coordinates + shift_x = np.random.uniform(-0.5, 0.5, size=len(source_catalog)) + shift_y = np.random.uniform(-0.5, 0.5, size=len(source_catalog)) + source_catalog["xcentroid"] += shift_x + source_catalog["ycentroid"] += shift_y + # add random fraction of a pixel shifts to the PSF coordinates + shift_x = np.random.uniform(-0.5, 0.5, size=len(source_catalog)) + shift_y = np.random.uniform(-0.5, 0.5, size=len(source_catalog)) + source_catalog["x_psf"] += shift_x + source_catalog["y_psf"] += shift_y + + # calculate centroid world coordinates + centroid = img.meta.wcs.pixel_to_world( + source_catalog["xcentroid"], + source_catalog["ycentroid"], + ) + # calculate PSF world coordinates + psf = img.meta.wcs.pixel_to_world( + source_catalog["x_psf"], + source_catalog["y_psf"], + ) + # add world coordinates to catalog + source_catalog["ra_centroid"], source_catalog["dec_centroid"] = ( + centroid.ra.deg, + centroid.dec.deg, + ) + source_catalog["ra_psf"], source_catalog["dec_psf"] = ( + psf.ra.deg, + psf.dec.deg, + ) + # add units + source_catalog["ra_centroid"].unit = u.deg + source_catalog["dec_centroid"].unit = u.deg + source_catalog["ra_psf"].unit = u.deg + source_catalog["dec_psf"].unit = u.deg + + # add source catalog to SourceCatalogModel + source_catalog_model.source_catalog = source_catalog + + return source_catalog_model From 5ae21ea5b87dc9483634440b73f41b2016c53ae0 Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Thu, 22 Aug 2024 18:19:45 +0000 Subject: [PATCH 05/20] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- romancal/tweakreg/tests/test_tweakreg.py | 1 - 1 file changed, 1 deletion(-) diff --git a/romancal/tweakreg/tests/test_tweakreg.py b/romancal/tweakreg/tests/test_tweakreg.py index 3cb778081..4dd12fdfc 100644 --- a/romancal/tweakreg/tests/test_tweakreg.py +++ b/romancal/tweakreg/tests/test_tweakreg.py @@ -3,7 +3,6 @@ import os from io import StringIO from pathlib import Path -import random from typing import Tuple import numpy as np From 3a070b39c73b5051575e3e9389748bb29919ece9 Mon Sep 17 00:00:00 2001 From: "M. Teodoro" Date: Thu, 22 Aug 2024 14:29:20 -0400 Subject: [PATCH 06/20] Check style fix. --- romancal/tweakreg/tests/test_tweakreg.py | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/romancal/tweakreg/tests/test_tweakreg.py b/romancal/tweakreg/tests/test_tweakreg.py index 4dd12fdfc..5bc9878a2 100644 --- a/romancal/tweakreg/tests/test_tweakreg.py +++ b/romancal/tweakreg/tests/test_tweakreg.py @@ -6,6 +6,7 @@ from typing import Tuple import numpy as np +from numpy.random import default_rng import pytest import requests from astropy import coordinates as coord @@ -1269,13 +1270,14 @@ def setup_source_catalog_model(img): source_catalog["y_psf"] = source_catalog["ycentroid"] # add random fraction of a pixel shifts to the centroid coordinates - shift_x = np.random.uniform(-0.5, 0.5, size=len(source_catalog)) - shift_y = np.random.uniform(-0.5, 0.5, size=len(source_catalog)) + rng = default_rng() + shift_x = rng.uniform(-0.5, 0.5, size=len(source_catalog)) + shift_y = rng.uniform(-0.5, 0.5, size=len(source_catalog)) source_catalog["xcentroid"] += shift_x source_catalog["ycentroid"] += shift_y # add random fraction of a pixel shifts to the PSF coordinates - shift_x = np.random.uniform(-0.5, 0.5, size=len(source_catalog)) - shift_y = np.random.uniform(-0.5, 0.5, size=len(source_catalog)) + shift_x = rng.uniform(-0.5, 0.5, size=len(source_catalog)) + shift_y = rng.uniform(-0.5, 0.5, size=len(source_catalog)) source_catalog["x_psf"] += shift_x source_catalog["y_psf"] += shift_y From 938f7f676b375f2ded1d30a5ca88825a34702d27 Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Thu, 22 Aug 2024 18:31:17 +0000 Subject: [PATCH 07/20] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- romancal/tweakreg/tests/test_tweakreg.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/romancal/tweakreg/tests/test_tweakreg.py b/romancal/tweakreg/tests/test_tweakreg.py index 5bc9878a2..38f3b85f4 100644 --- a/romancal/tweakreg/tests/test_tweakreg.py +++ b/romancal/tweakreg/tests/test_tweakreg.py @@ -6,7 +6,6 @@ from typing import Tuple import numpy as np -from numpy.random import default_rng import pytest import requests from astropy import coordinates as coord @@ -19,6 +18,7 @@ from gwcs import coordinate_frames as cf from gwcs import wcs from gwcs.geometry import CartesianToSpherical, SphericalToCartesian +from numpy.random import default_rng from roman_datamodels import datamodels as rdm from roman_datamodels import maker_utils From 8f8e348288b15add8c175400ea41e0a0b319066d Mon Sep 17 00:00:00 2001 From: "M. Teodoro" Date: Mon, 16 Sep 2024 17:12:01 -0400 Subject: [PATCH 08/20] Address comments. --- romancal/tweakreg/tests/test_tweakreg.py | 90 +++++++++++++++++++----- romancal/tweakreg/tweakreg_step.py | 12 +--- 2 files changed, 77 insertions(+), 25 deletions(-) diff --git a/romancal/tweakreg/tests/test_tweakreg.py b/romancal/tweakreg/tests/test_tweakreg.py index 38f3b85f4..5f18fc443 100644 --- a/romancal/tweakreg/tests/test_tweakreg.py +++ b/romancal/tweakreg/tests/test_tweakreg.py @@ -1,6 +1,8 @@ import copy +import filecmp import json import os +import shutil from io import StringIO from pathlib import Path from typing import Tuple @@ -1232,18 +1234,80 @@ def test_update_source_catalog_coordinates(tmp_path, base_image): cat_dec_psf = cat.source_catalog["dec_psf"] # calculate world coords using tweaked WCS - expected_centroid = img.meta.wcs.pixel_to_world( + expected_centroid = img.meta.wcs( cat.source_catalog["xcentroid"], cat.source_catalog["ycentroid"] ) - expected_psf = img.meta.wcs.pixel_to_world( + expected_psf = img.meta.wcs( cat.source_catalog["x_psf"], cat.source_catalog["y_psf"] ) # compare coordinates (make sure tweaked WCS was applied to cat file coords) - np.testing.assert_array_equal(cat_ra_centroid, expected_centroid.ra.value) - np.testing.assert_array_equal(cat_dec_centroid, expected_centroid.dec.value) - np.testing.assert_array_equal(cat_ra_psf, expected_psf.ra.value) - np.testing.assert_array_equal(cat_dec_psf, expected_psf.dec.value) + np.testing.assert_array_equal(cat_ra_centroid, expected_centroid[0]) + np.testing.assert_array_equal(cat_dec_centroid, expected_centroid[1]) + np.testing.assert_array_equal(cat_ra_psf, expected_psf[0]) + np.testing.assert_array_equal(cat_dec_psf, expected_psf[1]) + + +def test_source_catalog_coordinates_have_changed(tmp_path, base_image): + """Test that the original catalog file content is different from the updated file.""" + + os.chdir(tmp_path) + + img = base_image(shift_1=1000, shift_2=1000) + add_tweakreg_catalog_attribute(tmp_path, img, catalog_filename="img_1") + + tweakreg = trs.TweakRegStep() + + # create SourceCatalogModel + source_catalog_model = setup_source_catalog_model(img) + + # save SourceCatalogModel + tweakreg.save_model( + source_catalog_model, + output_file="img_1.asdf", + suffix="cat", + force=True, + ) + # save original data + shutil.copy("img_1_cat.asdf", "img_1_cat_original.asdf") + + # update tweakreg catalog name + img.meta.source_detection.tweakreg_catalog_name = "img_1_cat.asdf" + + # run TweakRegStep + res = trs.TweakRegStep.call([img]) + + # tweak the current WCS using TweakRegStep and save the updated cat file + with res: + dm = res.borrow(0) + assert dm.meta.source_detection.tweakreg_catalog_name == "img_1_cat.asdf" + tweakreg.update_catalog_coordinates( + dm.meta.source_detection.tweakreg_catalog_name, dm.meta.wcs + ) + res.shelve(dm, 0) + + cat_original = rdm.open("img_1_cat_original.asdf") + cat_updated = rdm.open("img_1_cat.asdf") + + # compare contents + assert not filecmp.cmp("img_1_cat.asdf", "img_1_cat_original.asdf") + # compare coordinates using rtol=1e-07 and atol=0 (default) + np.testing.assert_allclose( + cat_original.source_catalog["ra_centroid"], + cat_updated.source_catalog["ra_centroid"], + ) + np.testing.assert_allclose( + cat_original.source_catalog["dec_centroid"], + cat_updated.source_catalog["dec_centroid"], + ) + np.testing.assert_allclose( + cat_original.source_catalog["ra_psf"], + cat_updated.source_catalog["ra_psf"], + ) + np.testing.assert_allclose( + cat_original.source_catalog["dec_psf"], + cat_updated.source_catalog["dec_psf"], + ) def setup_source_catalog_model(img): @@ -1282,24 +1346,18 @@ def setup_source_catalog_model(img): source_catalog["y_psf"] += shift_y # calculate centroid world coordinates - centroid = img.meta.wcs.pixel_to_world( + centroid = img.meta.wcs( source_catalog["xcentroid"], source_catalog["ycentroid"], ) # calculate PSF world coordinates - psf = img.meta.wcs.pixel_to_world( + psf = img.meta.wcs( source_catalog["x_psf"], source_catalog["y_psf"], ) # add world coordinates to catalog - source_catalog["ra_centroid"], source_catalog["dec_centroid"] = ( - centroid.ra.deg, - centroid.dec.deg, - ) - source_catalog["ra_psf"], source_catalog["dec_psf"] = ( - psf.ra.deg, - psf.dec.deg, - ) + source_catalog["ra_centroid"], source_catalog["dec_centroid"] = centroid + source_catalog["ra_psf"], source_catalog["dec_psf"] = psf # add units source_catalog["ra_centroid"].unit = u.deg source_catalog["dec_centroid"].unit = u.deg diff --git a/romancal/tweakreg/tweakreg_step.py b/romancal/tweakreg/tweakreg_step.py index 7f151efd7..efd87b0ad 100644 --- a/romancal/tweakreg/tweakreg_step.py +++ b/romancal/tweakreg/tweakreg_step.py @@ -516,7 +516,7 @@ def update_catalog_coordinates(self, tweakreg_catalog_name, tweaked_wcs): ---------- tweakreg_catalog_name : str The name of the TweakReg catalog file produced by `SourceCatalog`. - tweaked_wcs : `astropy.wcs.WCS` + tweaked_wcs : `gwcs.wcs.WCS` The tweaked World Coordinate System (WCS) object. Returns @@ -539,17 +539,11 @@ def update_catalog_coordinates(self, tweakreg_catalog_name, tweaked_wcs): x_colname, y_colname = k ra_colname, dec_colname = v - # calculate new coordinates using tweaked WCS - tweaked_centroid = tweaked_wcs.pixel_to_world( + # calculate new coordinates using tweaked WCS and update catalog coordinates + catalog[ra_colname], catalog[dec_colname] = tweaked_wcs( catalog[x_colname], catalog[y_colname] ) - # update catalog coordinates - catalog[ra_colname], catalog[dec_colname] = ( - tweaked_centroid.ra.deg, - tweaked_centroid.dec.deg, - ) - # save updated catalog (overwrite cat file) self.save_model( source_catalog_model, From c052e600fbbfda20b7e3dcf899040a01f09f0b75 Mon Sep 17 00:00:00 2001 From: "M. Teodoro" Date: Mon, 16 Sep 2024 17:15:56 -0400 Subject: [PATCH 09/20] Add changelog entry. --- changes/1373.general.rst | 1 + 1 file changed, 1 insertion(+) create mode 100644 changes/1373.general.rst diff --git a/changes/1373.general.rst b/changes/1373.general.rst new file mode 100644 index 000000000..6c3b116b2 --- /dev/null +++ b/changes/1373.general.rst @@ -0,0 +1 @@ +Update source catalog file with tweaked coordinates. From a29e722bbb915780c2ed23a742ed0d727c074345 Mon Sep 17 00:00:00 2001 From: Zach Burnett Date: Tue, 17 Sep 2024 08:34:38 -0400 Subject: [PATCH 10/20] prune change log entry (moved to fragment in `changes/`) --- CHANGES.rst | 4 ---- 1 file changed, 4 deletions(-) diff --git a/CHANGES.rst b/CHANGES.rst index c55810b42..0cbd3e83f 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -19,10 +19,6 @@ resample 0.16.2 (2024-08-23) =================== -exposure_pipeline ------------------ -- Update source catalog file with tweaked coordinates. [#1373] - pipeline -------- - Added ``suffix`` to the spec of ExposurePipeline with a From e1d5c5729570b773c0d64d1447fcdbc3a8a82c92 Mon Sep 17 00:00:00 2001 From: Brett Date: Wed, 11 Sep 2024 10:28:31 -0400 Subject: [PATCH 11/20] unpin pytest --- pyproject.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pyproject.toml b/pyproject.toml index 10be0526a..58a53c15d 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -58,7 +58,7 @@ docs = [ ] test = [ "ci-watson >=0.5.0", - "pytest >=4.6.0, <8.0.0", + "pytest >8.0.0", "pytest-astropy", "deepdiff", ] From 7ff4c22d7c4127e9dc6e653fa4ebd5ecf639ca2a Mon Sep 17 00:00:00 2001 From: Brett Date: Wed, 11 Sep 2024 13:29:26 -0400 Subject: [PATCH 12/20] add lower pin to pytest-astropy --- pyproject.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pyproject.toml b/pyproject.toml index 58a53c15d..bca3c5fd4 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -59,7 +59,7 @@ docs = [ test = [ "ci-watson >=0.5.0", "pytest >8.0.0", - "pytest-astropy", + "pytest-astropy >= 0.11.0", "deepdiff", ] dev = [ From 9998daaae51c5e9b2367e12fa5b08f91e8df0bac Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Mon, 23 Sep 2024 14:30:33 +0000 Subject: [PATCH 13/20] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- romancal/tweakreg/tests/test_tweakreg.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/romancal/tweakreg/tests/test_tweakreg.py b/romancal/tweakreg/tests/test_tweakreg.py index 6d1085440..5bbf57e21 100644 --- a/romancal/tweakreg/tests/test_tweakreg.py +++ b/romancal/tweakreg/tests/test_tweakreg.py @@ -1181,6 +1181,7 @@ def setup_source_catalog_model(img): return source_catalog_model + @pytest.mark.parametrize( "exposure_type", ["WFI_GRISM", "WFI_PRISM", "WFI_DARK", "WFI_FLAT", "WFI_WFSC"], @@ -1264,4 +1265,3 @@ def test_tweakreg_handles_mixed_exposure_types(tmp_path, base_image): assert img3.meta.cal_step.tweakreg == "SKIPPED" assert img4.meta.cal_step.tweakreg == "COMPLETE" assert img5.meta.cal_step.tweakreg == "SKIPPED" - From b78a1fe0a70f5215ecd082d138ee1f8891705c13 Mon Sep 17 00:00:00 2001 From: "M. Teodoro" Date: Mon, 23 Sep 2024 17:32:24 -0400 Subject: [PATCH 14/20] Fix unit test. --- romancal/tweakreg/tests/test_tweakreg.py | 59 +++++++++++++++++++----- romancal/tweakreg/tweakreg_step.py | 1 + 2 files changed, 49 insertions(+), 11 deletions(-) diff --git a/romancal/tweakreg/tests/test_tweakreg.py b/romancal/tweakreg/tests/test_tweakreg.py index 5bbf57e21..955885341 100644 --- a/romancal/tweakreg/tests/test_tweakreg.py +++ b/romancal/tweakreg/tests/test_tweakreg.py @@ -11,7 +11,7 @@ import pytest import requests from astropy import coordinates as coord -from astropy import table +from astropy.table import Table from astropy import units as u from astropy.modeling import models from astropy.modeling.models import RotationSequence3D, Scale, Shift @@ -81,7 +81,7 @@ def create_custom_catalogs(tmp_path, base_image, catalog_format="ascii.ecsv"): # write line to catfile catfile_content.write(f"{x.get('cat_datamodel')} {x.get('cat_filename')}\n") # write out the catalog data - t = table.Table(x.get("cat_data"), names=("x", "y")) + t = Table(x.get("cat_data"), names=("x", "y")) t.write(tmp_path / x.get("cat_filename"), format=catalog_format) with open(catfile, mode="w") as f: print(catfile_content.getvalue(), file=f) @@ -380,7 +380,7 @@ def create_base_image_source_catalog( """ src_detector_coords = catalog_data output = os.path.join(tmp_path, output_filename) - t = table.Table(src_detector_coords, names=("x", "y")) + t = Table(src_detector_coords, names=("x", "y")) if save_catalogs: t.write((tmp_path / output), format=catalog_format) # mimic the same output format from SourceDetectionStep @@ -1101,25 +1101,62 @@ def test_source_catalog_coordinates_have_changed(tmp_path, base_image): cat_original = rdm.open("img_1_cat_original.asdf") cat_updated = rdm.open("img_1_cat.asdf") - # compare contents - assert not filecmp.cmp("img_1_cat.asdf", "img_1_cat_original.asdf") - # compare coordinates using rtol=1e-07 and atol=0 (default) - np.testing.assert_allclose( + # set max absolute and relative tolerance to ~ 1/2 a pixel + atol = u.Quantity(0.11 / 2, "arcsec").to("deg").value + rtol = 5e-8 + + # checking that nothing moved more than 1/2 a pixel + assert np.allclose( + cat_original.source_catalog["ra_centroid"], + cat_updated.source_catalog["ra_centroid"], + atol=atol, + rtol=rtol, + ) + assert np.allclose( + cat_original.source_catalog["dec_centroid"], + cat_updated.source_catalog["dec_centroid"], + atol=atol, + rtol=rtol, + ) + assert np.allclose( + cat_original.source_catalog["ra_psf"], + cat_updated.source_catalog["ra_psf"], + atol=atol, + rtol=rtol, + ) + assert np.allclose( + cat_original.source_catalog["dec_psf"], + cat_updated.source_catalog["dec_psf"], + atol=atol, + rtol=rtol, + ) + + # checking that things moved more than ~ 1/100 of a pixel + assert not np.allclose( cat_original.source_catalog["ra_centroid"], cat_updated.source_catalog["ra_centroid"], + atol=atol / 100, + rtol=rtol / 100, ) - np.testing.assert_allclose( + assert not np.allclose( cat_original.source_catalog["dec_centroid"], cat_updated.source_catalog["dec_centroid"], + atol=atol / 100, + rtol=rtol / 100, ) - np.testing.assert_allclose( + assert not np.allclose( cat_original.source_catalog["ra_psf"], cat_updated.source_catalog["ra_psf"], + atol=atol / 100, + rtol=rtol / 100, ) - np.testing.assert_allclose( + assert not np.allclose( cat_original.source_catalog["dec_psf"], cat_updated.source_catalog["dec_psf"], + atol=atol / 100, + rtol=rtol / 100, ) + assert True def setup_source_catalog_model(img): @@ -1228,7 +1265,7 @@ def test_tweakreg_skips_invalid_exposure_types(exposure_type, tmp_path, base_ima def test_validate_catalog_columns(catalog_data, expected_colnames, raises_exception): """Test that TweakRegStep._validate_catalog_columns() correctly validates the presence of required columns ('x' and 'y') in the provided catalog.""" - catalog = table.Table(catalog_data) + catalog = Table(catalog_data) if raises_exception: with pytest.raises(ValueError): _validate_catalog_columns(catalog) diff --git a/romancal/tweakreg/tweakreg_step.py b/romancal/tweakreg/tweakreg_step.py index d877e7971..988f15e55 100644 --- a/romancal/tweakreg/tweakreg_step.py +++ b/romancal/tweakreg/tweakreg_step.py @@ -57,6 +57,7 @@ class TweakRegStep(RomanStep): abs_nclip = integer(min=0, default=3) # Number of clipping iterations in fit when performing absolute astrometry abs_sigma = float(min=0.0, default=3.0) # Clipping limit in sigma units when performing absolute astrometry output_use_model = boolean(default=True) # When saving use `DataModel.meta.filename` + update_source_catalog_coordinates = boolean(default=False) # Update source catalog file with tweaked coordinates? """ # noqa: E501 reference_file_types = [] From bfd1b530658e51ddaf176e070a1fd4d27c9b7fdf Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Mon, 23 Sep 2024 21:32:56 +0000 Subject: [PATCH 15/20] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- romancal/tweakreg/tests/test_tweakreg.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/romancal/tweakreg/tests/test_tweakreg.py b/romancal/tweakreg/tests/test_tweakreg.py index 955885341..c3180b4df 100644 --- a/romancal/tweakreg/tests/test_tweakreg.py +++ b/romancal/tweakreg/tests/test_tweakreg.py @@ -1,5 +1,4 @@ import copy -import filecmp import json import os import shutil @@ -11,10 +10,10 @@ import pytest import requests from astropy import coordinates as coord -from astropy.table import Table from astropy import units as u from astropy.modeling import models from astropy.modeling.models import RotationSequence3D, Scale, Shift +from astropy.table import Table from astropy.time import Time from gwcs import coordinate_frames as cf from gwcs import wcs From 4e7d01249df86338794359852af9a97776edc20f Mon Sep 17 00:00:00 2001 From: "M. Teodoro" Date: Mon, 23 Sep 2024 17:40:45 -0400 Subject: [PATCH 16/20] Fix missed conflict code. --- romancal/tweakreg/tweakreg_step.py | 37 ------------------------------ 1 file changed, 37 deletions(-) diff --git a/romancal/tweakreg/tweakreg_step.py b/romancal/tweakreg/tweakreg_step.py index 988f15e55..9dbddd710 100644 --- a/romancal/tweakreg/tweakreg_step.py +++ b/romancal/tweakreg/tweakreg_step.py @@ -275,43 +275,6 @@ def process(self, input): return images - def get_tweakreg_catalog(self, images, i, image_model): - if hasattr(image_model.meta, "source_detection"): - is_tweakreg_catalog_present = hasattr( - image_model.meta.source_detection, "tweakreg_catalog" - ) - is_tweakreg_catalog_name_present = hasattr( - image_model.meta.source_detection, "tweakreg_catalog_name" - ) - if is_tweakreg_catalog_present: - # read catalog from structured array - catalog = Table( - np.asarray(image_model.meta.source_detection.tweakreg_catalog) - ) - elif is_tweakreg_catalog_name_present: - catalog = self.read_catalog( - image_model.meta.source_detection.tweakreg_catalog_name - ) - else: - images.shelve(image_model, i, modify=False) - raise AttributeError( - "Attribute 'meta.source_detection.tweakreg_catalog' is missing." - "Please either run SourceDetectionStep or provide a" - "custom source catalog." - ) - # remove 4D numpy array from meta.source_detection - if is_tweakreg_catalog_present: - del image_model.meta.source_detection["tweakreg_catalog"] - else: - images.shelve(image_model, i, modify=False) - raise AttributeError( - "Attribute 'meta.source_detection' is missing." - "Please either run SourceDetectionStep or provide a" - "custom source catalog." - ) - - return catalog - def update_catalog_coordinates(self, tweakreg_catalog_name, tweaked_wcs): """ Update the source catalog coordinates using the tweaked WCS. From 243f7d4967bca487e317baf7d05da87560705ecb Mon Sep 17 00:00:00 2001 From: "M. Teodoro" Date: Tue, 24 Sep 2024 06:54:14 -0400 Subject: [PATCH 17/20] Refactor unit test. --- romancal/tweakreg/tests/test_tweakreg.py | 51 ++++-------------------- 1 file changed, 8 insertions(+), 43 deletions(-) diff --git a/romancal/tweakreg/tests/test_tweakreg.py b/romancal/tweakreg/tests/test_tweakreg.py index c3180b4df..731c85e9d 100644 --- a/romancal/tweakreg/tests/test_tweakreg.py +++ b/romancal/tweakreg/tests/test_tweakreg.py @@ -1059,7 +1059,10 @@ def test_update_source_catalog_coordinates(tmp_path, base_image): np.testing.assert_array_equal(cat_dec_psf, expected_psf[1]) -def test_source_catalog_coordinates_have_changed(tmp_path, base_image): +@pytest.mark.parametrize( + "column_name", ["ra_centroid", "dec_centroid", "ra_psf", "dec_psf"] +) +def test_source_catalog_coordinates_have_changed(tmp_path, base_image, column_name): """Test that the original catalog file content is different from the updated file.""" os.chdir(tmp_path) @@ -1106,56 +1109,18 @@ def test_source_catalog_coordinates_have_changed(tmp_path, base_image): # checking that nothing moved more than 1/2 a pixel assert np.allclose( - cat_original.source_catalog["ra_centroid"], - cat_updated.source_catalog["ra_centroid"], - atol=atol, - rtol=rtol, - ) - assert np.allclose( - cat_original.source_catalog["dec_centroid"], - cat_updated.source_catalog["dec_centroid"], - atol=atol, - rtol=rtol, - ) - assert np.allclose( - cat_original.source_catalog["ra_psf"], - cat_updated.source_catalog["ra_psf"], - atol=atol, - rtol=rtol, - ) - assert np.allclose( - cat_original.source_catalog["dec_psf"], - cat_updated.source_catalog["dec_psf"], + cat_original.source_catalog[column_name], + cat_updated.source_catalog[column_name], atol=atol, rtol=rtol, ) - # checking that things moved more than ~ 1/100 of a pixel assert not np.allclose( - cat_original.source_catalog["ra_centroid"], - cat_updated.source_catalog["ra_centroid"], - atol=atol / 100, - rtol=rtol / 100, - ) - assert not np.allclose( - cat_original.source_catalog["dec_centroid"], - cat_updated.source_catalog["dec_centroid"], - atol=atol / 100, - rtol=rtol / 100, - ) - assert not np.allclose( - cat_original.source_catalog["ra_psf"], - cat_updated.source_catalog["ra_psf"], - atol=atol / 100, - rtol=rtol / 100, - ) - assert not np.allclose( - cat_original.source_catalog["dec_psf"], - cat_updated.source_catalog["dec_psf"], + cat_original.source_catalog[column_name], + cat_updated.source_catalog[column_name], atol=atol / 100, rtol=rtol / 100, ) - assert True def setup_source_catalog_model(img): From e93c63ca532c6aa16f56455a22ebe62b65ee07ba Mon Sep 17 00:00:00 2001 From: "M. Teodoro" Date: Tue, 24 Sep 2024 11:08:21 -0400 Subject: [PATCH 18/20] Always generate the same set of random shifts for unit test. --- romancal/tweakreg/tests/test_tweakreg.py | 65 +++++++++++++++++++----- 1 file changed, 52 insertions(+), 13 deletions(-) diff --git a/romancal/tweakreg/tests/test_tweakreg.py b/romancal/tweakreg/tests/test_tweakreg.py index 731c85e9d..9929e3393 100644 --- a/romancal/tweakreg/tests/test_tweakreg.py +++ b/romancal/tweakreg/tests/test_tweakreg.py @@ -1059,10 +1059,7 @@ def test_update_source_catalog_coordinates(tmp_path, base_image): np.testing.assert_array_equal(cat_dec_psf, expected_psf[1]) -@pytest.mark.parametrize( - "column_name", ["ra_centroid", "dec_centroid", "ra_psf", "dec_psf"] -) -def test_source_catalog_coordinates_have_changed(tmp_path, base_image, column_name): +def test_source_catalog_coordinates_have_changed(tmp_path, base_image): """Test that the original catalog file content is different from the updated file.""" os.chdir(tmp_path) @@ -1107,17 +1104,53 @@ def test_source_catalog_coordinates_have_changed(tmp_path, base_image, column_na atol = u.Quantity(0.11 / 2, "arcsec").to("deg").value rtol = 5e-8 - # checking that nothing moved more than 1/2 a pixel + # testing that nothing moved by more than 1/2 a pixel + assert np.allclose( + cat_original.source_catalog["ra_centroid"], + cat_updated.source_catalog["ra_centroid"], + atol=atol, + rtol=rtol, + ) assert np.allclose( - cat_original.source_catalog[column_name], - cat_updated.source_catalog[column_name], + cat_original.source_catalog["dec_centroid"], + cat_updated.source_catalog["dec_centroid"], atol=atol, rtol=rtol, ) - # checking that things moved more than ~ 1/100 of a pixel + assert np.allclose( + cat_original.source_catalog["ra_psf"], + cat_updated.source_catalog["ra_psf"], + atol=atol, + rtol=rtol, + ) + assert np.allclose( + cat_original.source_catalog["dec_psf"], + cat_updated.source_catalog["dec_psf"], + atol=atol, + rtol=rtol, + ) + # testing that things did move by more than ~ 1/100 of a pixel + assert not np.allclose( + cat_original.source_catalog["ra_centroid"], + cat_updated.source_catalog["ra_centroid"], + atol=atol / 100, + rtol=rtol / 100, + ) + assert not np.allclose( + cat_original.source_catalog["dec_centroid"], + cat_updated.source_catalog["dec_centroid"], + atol=atol / 100, + rtol=rtol / 100, + ) assert not np.allclose( - cat_original.source_catalog[column_name], - cat_updated.source_catalog[column_name], + cat_original.source_catalog["ra_psf"], + cat_updated.source_catalog["ra_psf"], + atol=atol / 100, + rtol=rtol / 100, + ) + assert not np.allclose( + cat_original.source_catalog["dec_psf"], + cat_updated.source_catalog["dec_psf"], atol=atol / 100, rtol=rtol / 100, ) @@ -1146,15 +1179,21 @@ def setup_source_catalog_model(img): source_catalog["x_psf"] = source_catalog["xcentroid"] source_catalog["y_psf"] = source_catalog["ycentroid"] - # add random fraction of a pixel shifts to the centroid coordinates - rng = default_rng() + # generate a set of random shifts to be added to the original coordinates + seed = 13 + rng = default_rng(seed) shift_x = rng.uniform(-0.5, 0.5, size=len(source_catalog)) shift_y = rng.uniform(-0.5, 0.5, size=len(source_catalog)) + # add random fraction of a pixel shifts to the centroid coordinates source_catalog["xcentroid"] += shift_x source_catalog["ycentroid"] += shift_y - # add random fraction of a pixel shifts to the PSF coordinates + + # generate another set of random shifts to be added to the original coordinates + seed = 5 + rng = default_rng(seed) shift_x = rng.uniform(-0.5, 0.5, size=len(source_catalog)) shift_y = rng.uniform(-0.5, 0.5, size=len(source_catalog)) + # add random fraction of a pixel shifts to the centroid coordinates source_catalog["x_psf"] += shift_x source_catalog["y_psf"] += shift_y From a6c03ec1e4dedc3991c3dee05d7bc6fd0addd53e Mon Sep 17 00:00:00 2001 From: mairan Date: Tue, 24 Sep 2024 12:05:19 -0400 Subject: [PATCH 19/20] Update CHANGES.rst --- CHANGES.rst | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGES.rst b/CHANGES.rst index 0cbd3e83f..e73e53971 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -21,6 +21,7 @@ resample pipeline -------- + - Added ``suffix`` to the spec of ExposurePipeline with a default value of ``cal``. Removed explicit setting of ``suffix`` so that it can be passed as an argument to ``strun``. [#1378] From 76f7f63ec38c4dc6d798ca593ac20944907150e3 Mon Sep 17 00:00:00 2001 From: mairan Date: Wed, 25 Sep 2024 14:01:30 -0400 Subject: [PATCH 20/20] Update 1373.general.rst --- changes/1373.general.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/changes/1373.general.rst b/changes/1373.general.rst index 6c3b116b2..359959479 100644 --- a/changes/1373.general.rst +++ b/changes/1373.general.rst @@ -1 +1 @@ -Update source catalog file with tweaked coordinates. +Update source catalog file with the tweaked coordinates.