Skip to content

Commit

Permalink
Add a check for separation and tolerance in tweakreg (#8476)
Browse files Browse the repository at this point in the history
  • Loading branch information
mcara committed Jun 5, 2024
1 parent 9b074a0 commit f3e13b1
Show file tree
Hide file tree
Showing 5 changed files with 55 additions and 5 deletions.
5 changes: 5 additions & 0 deletions CHANGES.rst
Original file line number Diff line number Diff line change
Expand Up @@ -217,6 +217,11 @@ tweakreg
- Change code default to use IRAF StarFinder instead of
DAO StarFinder [#8487]

- Added a check for ``(abs_)separation`` and ``(abs_)tolerance`` parameters
that ``separation`` > ``sqrt(2) * tolerance`` that will now log an error
message and skip ``tweakreg`` step when this condition is not satisfied and
source confusion is possible during catalog matching. [#8476]

wfss_contam
-----------

Expand Down
6 changes: 4 additions & 2 deletions docs/jwst/tweakreg/README.rst
Original file line number Diff line number Diff line change
Expand Up @@ -307,7 +307,8 @@ The ``tweakreg`` step has the following optional arguments:
* ``use2dhist``: A boolean indicating whether to use 2D histogram to find
initial offset. (Default=True)

* ``separation``: Minimum object separation in arcsec. (Default=1.0)
* ``separation``: Minimum object separation in arcsec. It **must be** at least
``sqrt(2)`` times larger than ``tolerance``. (Default=1.0)

* ``tolerance``: Matching tolerance for ``xyxymatch`` in arcsec. (Default=0.7)

Expand Down Expand Up @@ -374,7 +375,8 @@ Parameters used for absolute astrometry to a reference catalog.
Otherwise the initial guess for the offsets will be set to zero
(Default=True)

* ``abs_separation``: Minimum object separation in arcsec. (Default=1.0)
* ``abs_separation``: Minimum object separation in arcsec. It **must be** at
least ``sqrt(2)`` times larger than ``abs_tolerance``. (Default=1.0)

* ``abs_tolerance``: Matching tolerance for ``xyxymatch`` in arcsec.
(Default=0.7)
Expand Down
4 changes: 2 additions & 2 deletions jwst/regtest/test_niriss_image.py
Original file line number Diff line number Diff line change
Expand Up @@ -60,9 +60,9 @@ def test_niriss_tweakreg_no_sources(rtdata, fitsdiff_default_kwargs):
"--abs_refcat='GAIADR3'",
"--save_results=True",
]

# run the test from the command line:
result = Step.from_cmdline(args)
# Check that the step is skipped
assert result.skip

# Check the status of the step is set correctly in the files.
mc = datamodels.ModelContainer(rtdata.input)
Expand Down
21 changes: 20 additions & 1 deletion jwst/tweakreg/tests/test_tweakreg.py
Original file line number Diff line number Diff line change
Expand Up @@ -201,6 +201,25 @@ def test_tweakreg_step(example_input, with_shift):
assert abs_delta < 1E-12


@pytest.mark.parametrize("alignment_type", ['', 'abs_'])
def test_src_confusion_pars(example_input, alignment_type):
# assign images to different groups (so they are aligned to each other)
example_input[0].meta.group_id = 'a'
example_input[1].meta.group_id = 'b'

# make the step with arguments that may cause source confusion in match
pars = {
f"{alignment_type}separation": 1.0,
f"{alignment_type}tolerance": 1.0,
}
step = tweakreg_step.TweakRegStep(**pars)
result = step(example_input)

# check that step was skipped
for model in result:
assert model.meta.cal_step.tweakreg == 'SKIPPED'


@pytest.fixture()
def custom_catalog_path(tmp_path):
fn = tmp_path / "custom_catalog.ecsv"
Expand Down Expand Up @@ -286,7 +305,7 @@ def test_custom_catalog(custom_catalog_path, example_input, catfile, asn, meta,
elif catfile == "invalid_catfile":
pass

# figure out how many sources to expect for the model in group 'a'
# figure out how many sources to expect for the model in group 'a'
n_custom_sources = N_EXAMPLE_SOURCES
if custom:
if catfile == "valid_catfile":
Expand Down
24 changes: 24 additions & 0 deletions jwst/tweakreg/tweakreg_step.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
"""
from os import path
import math

from astropy import units as u
from astropy.coordinates import SkyCoord
Expand All @@ -23,6 +24,9 @@
from .tweakreg_catalog import make_tweakreg_catalog


_SQRT2 = math.sqrt(2.0)


def _oxford_or_str_join(str_list):
nelem = len(str_list)
if not nelem:
Expand Down Expand Up @@ -124,6 +128,26 @@ class TweakRegStep(Step):
def process(self, input):
images = ModelContainer(input)

if self.separation <= _SQRT2 * self.tolerance:
self.log.error(
"Parameter 'separation' must be larger than 'tolerance' by at "
"least a factor of sqrt(2) to avoid source confusion."
)
for model in images:
model.meta.cal_step.tweakreg = "SKIPPED"
self.log.warning("Skipping 'TweakRegStep' step.")
return input

if self.abs_separation <= _SQRT2 * self.abs_tolerance:
self.log.error(
"Parameter 'abs_separation' must be larger than 'abs_tolerance' "
"by at least a factor of sqrt(2) to avoid source confusion."
)
for model in images:
model.meta.cal_step.tweakreg = "SKIPPED"
self.log.warning("Skipping 'TweakRegStep' step.")
return input

if len(images) == 0:
raise ValueError("Input must contain at least one image model.")

Expand Down

0 comments on commit f3e13b1

Please sign in to comment.