Skip to content

Add drift_pars kwarg to get_aca_offsets and get_fid_offset #189

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Mar 19, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .pre-commit-config.yaml
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
repos:
- repo: https://github.com/astral-sh/ruff-pre-commit
# Ruff version.
rev: v0.9.0
rev: v0.9.9
hooks:
# Run the linter.
- id: ruff
Expand Down
33 changes: 27 additions & 6 deletions chandra_aca/drift.py
Original file line number Diff line number Diff line change
Expand Up @@ -171,7 +171,11 @@ def calc(self, times, t_ccd):
return out[0] if is_scalar else out


def get_fid_offset(time: CxoTimeLike, t_ccd: float) -> tuple:
def get_fid_offset(
time: CxoTimeLike,
t_ccd: float,
drift_pars: dict | None = None,
) -> tuple:
"""
Compute the fid light offset values for a given time and temperature.

Expand All @@ -183,6 +187,8 @@ def get_fid_offset(time: CxoTimeLike, t_ccd: float) -> tuple:
Time for offset calculation.
t_ccd : float
ACA CCD temperature in degrees Celsius.
drift_pars : dict, optional
ACA drift model parameters. If not supplied, the default parameters are used.

Returns
-------
Expand All @@ -201,13 +207,15 @@ def get_fid_offset(time: CxoTimeLike, t_ccd: float) -> tuple:
2022-11 aimpoint drift model and the FEB07 fid characteristics.
See https://github.com/sot/fid_drift_mon/blob/master/fid_offset_coeff.ipynb
"""
if drift_pars is None:
drift_pars = DRIFT_PARS

# Clip the time to the minimum time in the drift model
time = CxoTime(time).secs.clip(CxoTime("2012:001:12:00:00.000").secs, None)

# Define model instances using calibrated parameters
drift_y = AcaDriftModel(**DRIFT_PARS["dy"])
drift_z = AcaDriftModel(**DRIFT_PARS["dz"])
drift_y = AcaDriftModel(**drift_pars["dy"])
drift_z = AcaDriftModel(**drift_pars["dz"])

# Compute the predicted asol DY/DZ based on time and ACA CCD temperature
# via the predictive model calibrated in the fit_aimpoint_drift notebook
Expand All @@ -222,7 +230,15 @@ def get_fid_offset(time: CxoTimeLike, t_ccd: float) -> tuple:
return dy_pred + y_offset, dz_pred + z_offset


def get_aca_offsets(detector, chip_id, chipx, chipy, time, t_ccd):
def get_aca_offsets(
detector,
chip_id,
chipx,
chipy,
time,
t_ccd,
drift_pars: dict | None = None,
):
"""
Compute the dynamical ACA offset values for the provided inputs.

Expand All @@ -242,14 +258,19 @@ def get_aca_offsets(detector, chip_id, chipx, chipy, time, t_ccd):
time(s) of observation (any Chandra.Time compatible format)
t_ccd
ACA CCD temperature(s) (degC)
drift_pars : dict, optional
ACA drift model parameters. If not supplied, the default parameters are used.

Returns
-------
aca_offset_y, aca_offset_z (arcsec)
"""
if drift_pars is None:
drift_pars = DRIFT_PARS

# Define model instances using calibrated parameters
drift_y = AcaDriftModel(**DRIFT_PARS["dy"])
drift_z = AcaDriftModel(**DRIFT_PARS["dz"])
drift_y = AcaDriftModel(**drift_pars["dy"])
drift_z = AcaDriftModel(**drift_pars["dz"])

try:
asol_to_chip = ASOL_TO_CHIP[detector, chip_id]
Expand Down
46 changes: 39 additions & 7 deletions chandra_aca/tests/test_drift.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,14 @@
import astropy.table as tbl
import numpy as np
import pytest
from ska_helpers.utils import LazyDict

from chandra_aca import drift

# Drift pars for testing that may reflect a new model in a non-flight testing repo.
# See test_get_aca_offsets() for how to test a new drift model.
DRIFT_PARS_TEST = LazyDict(drift.load_drift_pars)


def test_get_aca_offsets_legacy():
"""
Expand Down Expand Up @@ -53,13 +58,38 @@ def test_get_aca_offsets_legacy():


@pytest.mark.parametrize("kwargs", kwargs_list)
def test_get_aca_offsets(kwargs, monkeypatch):
"""Regression test that ACA offsets match the original flight values from 2022-11
analysis to expected precision."""
monkeypatch.setenv("CHANDRA_MODELS_DEFAULT_VERSION", "3.48")
@pytest.mark.parametrize("use_drift_pars", [True, False])
def test_get_aca_offsets(kwargs, use_drift_pars):
"""Regression test that ACA offsets match expected.

Expected values are the original flight values from 2022-11 analysis to expected
precision. The chandra_models version is (by default) current flight, so this test
ensures that any model updates do not introduce a regression.

Testing a new drift model here can be done with environment variables::

env CHANDRA_MODELS_REPO_DIR=$HOME/git/chandra_models \
CHANDRA_MODELS_DEFAULT_VERSION=aimpoint-drift-update \
pytest -k test_drift

When a new drift model is released, the `aimpoint_regression_data.ecsv` file can
optionally be updated with the new values from the analysis notebook.
"""
kwargs = kwargs.copy()
aca_offset_y = kwargs.pop("aca_offset_y")
aca_offset_z = kwargs.pop("aca_offset_z")

# Test explicitly providing drift_pars. This should be exactly the same as the
# default handling.
if use_drift_pars:
kwargs["drift_pars"] = DRIFT_PARS_TEST
# Print info for use with `-s` flag to see what model is being used.
info = DRIFT_PARS_TEST["info"]
repo_path = info["repo_path"]
version = info["version"]
md5 = info["md5"]
print(f"{repo_path=} {version=} {md5=}")

offsets = drift.get_aca_offsets(**kwargs)
dy = offsets[0] - aca_offset_y
dz = offsets[1] - aca_offset_z
Expand All @@ -70,19 +100,21 @@ def test_get_aca_offsets(kwargs, monkeypatch):


@pytest.mark.parametrize(
"t, t_ccd, expected_dy, expected_dz",
"date, t_ccd, expected_dy, expected_dz",
[
("2018:284", -10.0, -7.50, -5.30),
("2018:286", -10.0, 5.01, 0.75),
("2022:293", -10, 9.46, 1.39),
("2022:295", -10, 17.44, 2.90),
],
)
def test_get_fid_offset(t, t_ccd, expected_dy, expected_dz):
@pytest.mark.parametrize("use_drift_pars", [True, False])
def test_get_fid_offset(date, t_ccd, expected_dy, expected_dz, use_drift_pars):
"""
Test that the get_fid_offset function returns expected values for a few inputs.
"""
dy, dz = drift.get_fid_offset(t, t_ccd)
kwargs = {"drift_pars": DRIFT_PARS_TEST} if use_drift_pars else {}
dy, dz = drift.get_fid_offset(date, t_ccd, **kwargs)
assert np.isclose(dy, expected_dy, atol=0.01)
assert np.isclose(dz, expected_dz, atol=0.01)

Expand Down