From 057152bc96a187b175d636f9fc9f5343c816dd25 Mon Sep 17 00:00:00 2001 From: Joseph Spitale Date: Mon, 10 Aug 2026 12:21:04 -0700 Subject: [PATCH 01/22] Add custom C-matrix support to Cassini ISS from_file Add cmatrix/frame_id/map_other_camera arguments to hosts/cassini/iss.py from_file, letting a caller supply a C-matrix (J2000 -> camera image frame) instead of SPICE pointing for the camera named in the label. Optionally derive and register the co-mounted other camera's frame from the fixed inter-camera rotation. The override lives in a new ISS.set_cmatrix method; the SPICE-derived camera frames are now built lazily by ISS.define_camera_frames() so a custom C-matrix without mapping never depends on SPICE (no CK loaded). Also add an override option to oops/frame/cmatrix.py Cmatrix so it can replace an already-registered frame. Co-Authored-By: Claude Opus 4.8 --- oops/frame/cmatrix.py | 6 +- oops/hosts/cassini/iss.py | 118 ++++++++++++++++++++++++++++++++++++-- 2 files changed, 116 insertions(+), 8 deletions(-) diff --git a/oops/frame/cmatrix.py b/oops/frame/cmatrix.py index ffad650f..6474e294 100755 --- a/oops/frame/cmatrix.py +++ b/oops/frame/cmatrix.py @@ -21,7 +21,7 @@ class Cmatrix(Frame): # expendable. Frame IDs are not preserved during pickling. #=========================================================================== - def __init__(self, cmatrix, reference=None, frame_id=None): + def __init__(self, cmatrix, reference=None, frame_id=None, override=False): """Constructor for a Cmatrix frame. Input: @@ -30,6 +30,8 @@ def __init__(self, cmatrix, reference=None, frame_id=None): None for J2000. frame_id the ID under which the frame will be registered; None to leave the frame unregistered + override if True, replace the primary definition of any frame + already registered under this frame_id. """ self.cmatrix = Matrix3.as_matrix3(cmatrix) @@ -42,7 +44,7 @@ def __init__(self, cmatrix, reference=None, frame_id=None): self.keys = set() # Update wayframe and frame_id; register if not temporary - self.register() + self.register(override=override) # It needs a wayframe before we can construct the transform self.transform = Transform(cmatrix, Vector3.ZERO, diff --git a/oops/hosts/cassini/iss.py b/oops/hosts/cassini/iss.py index 5fbc310c..bc70df69 100755 --- a/oops/hosts/cassini/iss.py +++ b/oops/hosts/cassini/iss.py @@ -18,7 +18,8 @@ ################################################################################ def from_file(filespec, fast_distortion=True, - return_all_planets=False, **parameters): + return_all_planets=False, cmatrix=None, frame_id=None, + map_other_camera=False, **parameters): """A general, static method to return a Snapshot object based on a given Cassini ISS image file. @@ -31,6 +32,21 @@ def from_file(filespec, fast_distortion=True, return_all_planets Include kernels for all planets not just Jupiter or Saturn. + + cmatrix an oops.Matrix3 (or 3x3 array) giving the rotation + from J2000 into the camera image frame, used instead + of SPICE pointing. None uses SPICE as before. + + frame_id when cmatrix is given and map_other_camera is False, + register the C-matrix under this frame ID and use only + that frame; None overrides the global + CASSINI_ISS_ frame. + + map_other_camera when True, also derive and register the other camera's + frame from the fixed inter-camera rotation (uses + SPICE); frame_id is ignored. When False (default), + only the label camera's frame is created and SPICE + pointing is completely circumvented (no CK loaded). """ ISS.initialize() # Define everything the first time through; use defaults @@ -67,15 +83,30 @@ def from_file(filespec, fast_distortion=True, elif vicar_dict['GAIN_MODE_ID'][:2] == '12': gain_mode = 3 - # Make sure the SPICE kernels are loaded - Cassini.load_cks( tstart, tstart + texp) + # Make sure the SPICE kernels are loaded. SPKs are always needed for the + # spacecraft path; CKs (pointing) are only needed when the pointing comes + # from SPICE, i.e. no custom cmatrix, or when mapping to the other camera. Cassini.load_spks(tstart, tstart + texp) + if cmatrix is None or map_other_camera: + Cassini.load_cks(tstart, tstart + texp) + + # Determine the observation frame, applying a custom C-matrix if given. + # ISS.set_cmatrix registers the frame(s); SPICE pointing is circumvented + # unless the other camera is being mapped. + frame = 'CASSINI_ISS_' + camera + if cmatrix is not None: + ISS.set_cmatrix(cmatrix, camera, frame_id=frame_id, + map_other_camera=map_other_camera, time=tstart) + if frame_id is not None and not map_other_camera: + frame = frame_id + else: + ISS.define_camera_frames() # use the SPICE-derived pointing # Create a Snapshot result = oops.obs.Snapshot(('v','u'), tstart, texp, ISS.fovs[camera,mode, fast_distortion], path = 'CASSINI', - frame = 'CASSINI_ISS_' + camera, + frame = frame, dict = vicar_dict, # Add the VICAR dict data = vic.data_2d, # Add the data array instrument = 'ISS', @@ -101,6 +132,7 @@ def from_index(filespec, **parameters): label of the index file. """ ISS.initialize() # Define everything the first time through + ISS.define_camera_frames() # use the SPICE-derived pointing filespec = FCPath(filespec) @@ -186,6 +218,8 @@ class ISS(object): instrument_kernel = None fovs = {} initialized = False + frames_defined = False + offset_wac = False # Create a master version of the NAC and WAC distortion models from # Owen Jr., W.M., 2003. Cassini ISS Geometric Calibration of April 2003. @@ -374,6 +408,27 @@ def initialize(ck='reconstructed', planets=None, offset_wac=False, asof=None, ISS.fovs[detector, 'SUM2'] = oops.fov.SubsampledFOV(full_fov_none, 2) ISS.fovs[detector, 'SUM4'] = oops.fov.SubsampledFOV(full_fov_none, 4) + # Remember the WAC offset option. The SPICE-derived camera frames are + # built lazily by define_camera_frames(), so an observation that supplies + # a custom C-matrix (without mapping) never depends on SPICE pointing. + ISS.offset_wac = offset_wac + + ISS.initialized = True + + #=========================================================================== + @staticmethod + def define_camera_frames(): + """Register the SPICE-derived CASSINI_ISS_NAC and CASSINI_ISS_WAC frames. + + ISS.initialize() must have been called first. Built lazily (and only + once) so that observations using a custom C-matrix without mapping never + construct or depend on the SPICE camera frames. + """ + + # Quick exit after first call + if ISS.frames_defined: + return + # Construct a SpiceFrame for each camera # Deal with the fact that the instrument's internal # coordinate system is rotated 180 degrees @@ -385,7 +440,7 @@ def initialize(ck='reconstructed', planets=None, offset_wac=False, asof=None, nac_frame = oops.frame.Cmatrix(rot180, nac_flipped, frame_id='CASSINI_ISS_NAC') - if offset_wac: + if ISS.offset_wac: # Apply offset for WAC relative to NAC info = ISS.instrument_kernel['INS']['CASSINI_ISS_NAC'] @@ -408,7 +463,56 @@ def initialize(ck='reconstructed', planets=None, offset_wac=False, asof=None, wac_frame = oops.frame.Cmatrix(rot180, wac_flipped, frame_id='CASSINI_ISS_WAC') - ISS.initialized = True + ISS.frames_defined = True + + #=========================================================================== + @staticmethod + def set_cmatrix(cmatrix, camera, frame_id=None, map_other_camera=False, + time=None): + """Register a custom C-matrix as an ISS observation's pointing, replacing + the SPICE-derived camera frame. + + ISS.initialize() must have been called first. The C-matrix gives the + rotation from J2000 into the camera image frame (X right, Y down, Z along + the optic axis). + + Input: + cmatrix an oops.Matrix3 (or 3x3 array) for the named camera. + camera 'NAC' or 'WAC', the camera the cmatrix applies to. + frame_id when map_other_camera is False, register the C-matrix + under this frame ID; None overrides the global + CASSINI_ISS_ frame. + map_other_camera when True, also derive and register the other + camera's frame from the fixed inter-camera rotation (uses + SPICE); frame_id is ignored. + time the observation time (TDB), required to evaluate the + inter-camera rotation when map_other_camera is True. + """ + + cmatrix = oops.Matrix3.as_matrix3(cmatrix) + + if map_other_camera: + # rel = M_other<-label, the fixed rotation read from the + # SPICE-derived frames at the observation time. + ISS.define_camera_frames() + other = 'WAC' if camera == 'NAC' else 'NAC' + label_wf = oops.frame.Frame.as_wayframe('CASSINI_ISS_' + camera) + other_wf = oops.frame.Frame.as_wayframe('CASSINI_ISS_' + other) + rel = other_wf.wrt(label_wf).transform_at_time(time).matrix + + oops.frame.Cmatrix(cmatrix, frame_id='CASSINI_ISS_' + camera, + override=True) + oops.frame.Cmatrix(rel * cmatrix, + frame_id='CASSINI_ISS_' + other, override=True) + + elif frame_id is not None: + # Single custom frame; global frames untouched + oops.frame.Cmatrix(cmatrix, frame_id=frame_id) + + else: + # Override the label camera's global frame + oops.frame.Cmatrix(cmatrix, frame_id='CASSINI_ISS_' + camera, + override=True) #=========================================================================== @staticmethod @@ -421,6 +525,8 @@ def reset(): ISS.instrument_kernel = None ISS.fovs = {} ISS.initialized = False + ISS.frames_defined = False + ISS.offset_wac = False Cassini.reset() From cdad9ca7acd5e23f9a8cf6fcac722b5d363b62ad Mon Sep 17 00:00:00 2001 From: Joseph Spitale Date: Mon, 10 Aug 2026 14:52:24 -0700 Subject: [PATCH 02/22] Add Observation.cmatrix() to derive the J2000-to-frame rotation Generic getter on the Observation base class that evaluates self.frame's rotation relative to a reference frame (default J2000) at a time picked from a (u,v) location (default the FOV center), via the existing midtime_at_uv(). Returns the raw Matrix3 so it round-trips with the cmatrix inputs accepted by ISS.from_file()/ISS.set_cmatrix. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01WWmXpTiygmBh786ED6GigN --- oops/observation/observation_.py | 34 ++++++++++++++++++++++++++++++ tests/observation/test_snapshot.py | 16 +++++++++++++- 2 files changed, 49 insertions(+), 1 deletion(-) diff --git a/oops/observation/observation_.py b/oops/observation/observation_.py index 989781f7..ac4d3244 100755 --- a/oops/observation/observation_.py +++ b/oops/observation/observation_.py @@ -8,6 +8,7 @@ from polymath import Scalar, Pair, Vector, Vector3, Qube from oops.config import LOGGING, PATH_PHOTONS from oops.event import Event +from oops.frame import Frame from oops.frame.navigation import Navigation from oops.meshgrid import Meshgrid @@ -450,6 +451,39 @@ def midtime_at_uv(self, uv, tfrac=0.5): (time0, time1) = self.time_range_at_uv(uv) return tfrac * (time0 + time1) + #=========================================================================== + def cmatrix(self, uv=None, time=None, reference=None): + """The 3x3 rotation matrix that rotates a reference frame into this + observation's frame at a selected time. + + This is the same J2000 -> camera-frame rotation accepted as a + "cmatrix" input elsewhere in this package (e.g., + oops.hosts.cassini.iss.from_file(cmatrix=...)), so the result of this + method can be fed back in to reproduce this observation's pointing. + + Input: + uv a (u,v) pixel location, used only to select a time + when this observation's frame is time-dependent (e.g., + a slewing spacecraft during a raster or TDI exposure); + None to use the center of the FOV. Ignored if time is + given explicitly. + time the time in seconds TDB at which to evaluate the + frame; None to derive it from uv via midtime_at_uv(). + reference the frame or frame ID that the returned matrix rotates + from; None for J2000. + + Return: a Matrix3 giving the rotation from the reference frame + into this observation's frame at the selected time. + """ + + if time is None: + uv = self.fov.uv_shape / 2. if uv is None else Pair.as_pair(uv) + time = self.midtime_at_uv(uv) + + reference = Frame.as_wayframe(reference) or Frame.J2000 + xform = self.frame.wrt(reference).transform_at_time(time) + return xform.matrix + #=========================================================================== def meshgrid(self, origin=None, undersample=1, oversample=1, limit=None, center_uv=None, fov_keywords={}): diff --git a/tests/observation/test_snapshot.py b/tests/observation/test_snapshot.py index cc71e8bf..8d821a81 100755 --- a/tests/observation/test_snapshot.py +++ b/tests/observation/test_snapshot.py @@ -5,8 +5,9 @@ import numpy as np import unittest -from polymath import Pair, Vector +from polymath import Matrix3, Pair, Vector from oops.fov import FlatFOV +from oops.frame import Cmatrix from oops.observation import Snapshot @@ -124,6 +125,19 @@ def runTest(self): self.assertEqual(uv[:4], indices.to_pair((2,0))[:4]) self.assertTrue(np.all(uv.mask == 4*[False] + [True])) + # cmatrix() + m = Matrix3([[0,1,0],[0,0,-1],[-1,0,0]]) + cmatrix_frame = Cmatrix(m, frame_id='TEST_SNAPSHOT_CMATRIX') + + obs = Snapshot(('u','v'), tstart=98., texp=2., + fov=fov, path='SSB', frame=cmatrix_frame) + + self.assertTrue(np.all(obs.cmatrix().vals == m.vals)) + self.assertTrue(np.all(obs.cmatrix(uv=(3,4)).vals == m.vals)) + self.assertTrue(np.all(obs.cmatrix(time=99.).vals == m.vals)) + self.assertTrue(np.all(obs.cmatrix(reference=obs.frame).vals + == np.eye(3))) + ################################################################################ if __name__ == '__main__': unittest.main(verbosity=2) From 4ab0943a3114e369514128707134a5841dbe043a Mon Sep 17 00:00:00 2001 From: Joseph Spitale Date: Mon, 10 Aug 2026 15:50:53 -0700 Subject: [PATCH 03/22] Use dedicated SPICE frames for inter-camera mapping; extend cmatrix tests Compute the map_other_camera inter-camera rotation from dedicated, immutable CASSINI_ISS__SPICE frames instead of the mutable CASSINI_ISS_ frames that set_cmatrix can override. A single-camera override followed by a mapped-camera load would otherwise derive `rel` from the overridden custom frame, corrupting the mapped camera's pointing. Add a regression test (test_map_after_single_camera_override) covering that ordering, wire the ISS Cmatrix test module into the unittester, and extend the Snapshot cmatrix() test to use a time-dependent SpinFrame and a uv-dependent midtime so it verifies cmatrix() actually consults both uv and time. Co-Authored-By: Claude Opus 4.8 --- oops/hosts/cassini/iss.py | 49 +++++-- tests/hosts/cassini/iss/__init__.py | 2 + tests/hosts/cassini/iss/test_iss.py | 192 ++++++++++++++++++++++++++ tests/hosts/cassini/iss/unittester.py | 1 + tests/observation/test_snapshot.py | 43 +++++- 5 files changed, 276 insertions(+), 11 deletions(-) create mode 100644 tests/hosts/cassini/iss/test_iss.py diff --git a/oops/hosts/cassini/iss.py b/oops/hosts/cassini/iss.py index bc70df69..530d4a65 100755 --- a/oops/hosts/cassini/iss.py +++ b/oops/hosts/cassini/iss.py @@ -437,8 +437,6 @@ def define_camera_frames(): frame_id='CASSINI_ISS_NAC_FLIPPED') wac_flipped = oops.frame.SpiceFrame('CASSINI_ISS_WAC', frame_id='CASSINI_ISS_WAC_FLIPPED') - nac_frame = oops.frame.Cmatrix(rot180, nac_flipped, - frame_id='CASSINI_ISS_NAC') if ISS.offset_wac: @@ -455,13 +453,41 @@ def define_camera_frames(): # This is Rob's determination of WAC - NAC in units of NAC pixels xshift = -7. * xpixel yshift = 4.4 * ypixel + + # Dedicated SPICE-derived image frames, kept under their own frame IDs. + # set_cmatrix only ever overrides CASSINI_ISS_NAC/WAC, so these *_SPICE + # frames are never replaced by a custom C-matrix and remain a reliable + # reference for the inter-camera rotation (see set_cmatrix). override=True + # so a rebuild after ISS.reset() refreshes them to the current pointing. + oops.frame.Cmatrix(rot180, nac_flipped, + frame_id='CASSINI_ISS_NAC_SPICE', override=True) + if ISS.offset_wac: + wac_spice_no = oops.frame.Cmatrix(rot180, wac_flipped, + frame_id='CASSINI_ISS_WAC-NO_OFFSET_SPICE', + override=True) + oops.frame.Navigation((xshift,yshift), wac_spice_no, + frame_id='CASSINI_ISS_WAC_SPICE', override=True) + else: + oops.frame.Cmatrix(rot180, wac_flipped, + frame_id='CASSINI_ISS_WAC_SPICE', override=True) + + # Observation-facing camera frames. These start out identical to the + # *_SPICE frames above, but set_cmatrix may later replace them with a + # custom C-matrix. override=True: a prior custom C-matrix may already + # have replaced the primary definition of this frame ID; reclaim it. + nac_frame = oops.frame.Cmatrix(rot180, nac_flipped, + frame_id='CASSINI_ISS_NAC', override=True) + + if ISS.offset_wac: wac_frame_no = oops.frame.Cmatrix(rot180, wac_flipped, - frame_id='CASSINI_ISS_WAC-NO_OFFSET') + frame_id='CASSINI_ISS_WAC-NO_OFFSET', + override=True) wac_frame = oops.frame.Navigation((xshift,yshift), wac_frame_no, - frame_id='CASSINI_ISS_WAC') + frame_id='CASSINI_ISS_WAC', + override=True) else: wac_frame = oops.frame.Cmatrix(rot180, wac_flipped, - frame_id='CASSINI_ISS_WAC') + frame_id='CASSINI_ISS_WAC', override=True) ISS.frames_defined = True @@ -492,12 +518,17 @@ def set_cmatrix(cmatrix, camera, frame_id=None, map_other_camera=False, cmatrix = oops.Matrix3.as_matrix3(cmatrix) if map_other_camera: - # rel = M_other<-label, the fixed rotation read from the - # SPICE-derived frames at the observation time. + # rel = M_other<-label, the fixed rotation read from the dedicated + # *_SPICE frames at the observation time. These are used (rather than + # CASSINI_ISS_NAC/WAC) because a prior set_cmatrix override may have + # replaced the CASSINI_ISS_ frames with a custom C-matrix, + # which would corrupt the inter-camera rotation. ISS.define_camera_frames() other = 'WAC' if camera == 'NAC' else 'NAC' - label_wf = oops.frame.Frame.as_wayframe('CASSINI_ISS_' + camera) - other_wf = oops.frame.Frame.as_wayframe('CASSINI_ISS_' + other) + label_wf = oops.frame.Frame.as_wayframe('CASSINI_ISS_' + camera + + '_SPICE') + other_wf = oops.frame.Frame.as_wayframe('CASSINI_ISS_' + other + + '_SPICE') rel = other_wf.wrt(label_wf).transform_at_time(time).matrix oops.frame.Cmatrix(cmatrix, frame_id='CASSINI_ISS_' + camera, diff --git a/tests/hosts/cassini/iss/__init__.py b/tests/hosts/cassini/iss/__init__.py index 636e24d6..c466e90a 100755 --- a/tests/hosts/cassini/iss/__init__.py +++ b/tests/hosts/cassini/iss/__init__.py @@ -8,6 +8,8 @@ from oops.body import Body from oops.unittester_support import TEST_DATA_PREFIX +from tests.hosts.cassini.iss.test_iss import Test_Cassini_ISS_Cmatrix + class Test_Cassini_ISS_GoldMaster(unittest.TestCase): diff --git a/tests/hosts/cassini/iss/test_iss.py b/tests/hosts/cassini/iss/test_iss.py new file mode 100644 index 00000000..ca553a41 --- /dev/null +++ b/tests/hosts/cassini/iss/test_iss.py @@ -0,0 +1,192 @@ +################################################################################ +# tests/hosts/cassini/iss/test_iss.py +################################################################################ + +import numpy as np +import unittest + +import oops +from oops.body import Body +from oops.frame import Frame +from oops.hosts.cassini import Cassini +from oops.hosts.cassini.iss import ISS, from_file +from oops.unittester_support import TEST_DATA_PREFIX + +# A rotation distinguishable from any SPICE-derived pointing, used to build +# fake "custom" C-matrices below. +_PERTURBATION = oops.Matrix3([[0,1,0],[-1,0,0],[0,0,1]]) + + +class Test_Cassini_ISS_Cmatrix(unittest.TestCase): + """Tests for the custom C-matrix support in hosts/cassini/iss.py + (from_file's cmatrix/frame_id/map_other_camera arguments and + ISS.set_cmatrix), plus the generic Observation.cmatrix() getter. + """ + + FILESPEC = 'cassini/ISS/W1573721822_1.IMG' # a WAC image + + def setUp(self): + # Custom C-matrices can override the global CASSINI_ISS_NAC/WAC + # frames, so every test starts from a clean registry. + Body.reset_registry() + Body.define_solar_system() + ISS.reset() + + self.filespec = TEST_DATA_PREFIX.retrieve(self.FILESPEC) + + def tearDown(self): + Body.reset_registry() + Body.define_solar_system() + ISS.reset() + + #=========================================================================== + def test_default_override(self): + """A custom cmatrix with no frame_id overrides the global camera + frame, and Observation.cmatrix() round-trips it.""" + + baseline = from_file(self.filespec) + camera = baseline.dict['INSTRUMENT_ID'][3:] + 'C' + custom = _PERTURBATION * baseline.cmatrix() + + obs = from_file(self.filespec, cmatrix=custom) + + self.assertEqual(obs.frame.frame_id, 'CASSINI_ISS_' + camera) + self.assertTrue(np.all(obs.cmatrix().vals == custom.vals)) + + # The global frame really was replaced, not merely shadowed + direct = (Frame.as_wayframe('CASSINI_ISS_' + camera) + .wrt(Frame.J2000).transform_at_time(obs.tstart).matrix) + self.assertTrue(np.all(direct.vals == custom.vals)) + + #=========================================================================== + def test_explicit_frame_id_leaves_global_untouched(self): + """A custom cmatrix with an explicit frame_id registers a separate + frame and leaves the global CASSINI_ISS_ frame alone.""" + + baseline = from_file(self.filespec) + m0 = baseline.cmatrix() + custom = _PERTURBATION * m0 + + obs = from_file(self.filespec, cmatrix=custom, + frame_id='TEST_ISS_CUSTOM_FRAME') + + self.assertEqual(obs.frame.frame_id, 'TEST_ISS_CUSTOM_FRAME') + self.assertTrue(np.all(obs.cmatrix().vals == custom.vals)) + + # The global camera frame is untouched + unaffected = from_file(self.filespec) + self.assertTrue(np.all(unaffected.cmatrix().vals == m0.vals)) + + #=========================================================================== + def test_map_other_camera(self): + """map_other_camera=True derives the co-mounted camera's frame from + the fixed, SPICE-derived inter-camera rotation, preserving it under a + custom C-matrix.""" + + baseline = from_file(self.filespec) + camera = baseline.dict['INSTRUMENT_ID'][3:] + 'C' + other = 'NAC' if camera == 'WAC' else 'WAC' + m0 = baseline.cmatrix() + + baseline_rel = (Frame.as_wayframe('CASSINI_ISS_' + other) + .wrt(Frame.as_wayframe('CASSINI_ISS_' + camera)) + .transform_at_time(baseline.tstart).matrix) + + custom = _PERTURBATION * m0 + obs = from_file(self.filespec, cmatrix=custom, map_other_camera=True) + + self.assertEqual(obs.frame.frame_id, 'CASSINI_ISS_' + camera) + self.assertTrue(np.all(obs.cmatrix().vals == custom.vals)) + + # The fixed inter-camera rotation is preserved under the override + new_rel = (Frame.as_wayframe('CASSINI_ISS_' + other) + .wrt(Frame.as_wayframe('CASSINI_ISS_' + camera)) + .transform_at_time(baseline.tstart).matrix) + self.assertTrue(np.allclose(new_rel.vals, baseline_rel.vals)) + + # ...and the other camera's absolute pointing was updated accordingly + other_matrix = (Frame.as_wayframe('CASSINI_ISS_' + other) + .wrt(Frame.J2000) + .transform_at_time(baseline.tstart).matrix) + self.assertTrue(np.allclose(other_matrix.vals, + (baseline_rel * custom).vals)) + + #=========================================================================== + def test_map_after_single_camera_override(self): + """A single-camera override followed by a mapped-camera load must still + derive the inter-camera rotation from SPICE, not from the overridden + CASSINI_ISS_ frame.""" + + baseline = from_file(self.filespec) + camera = baseline.dict['INSTRUMENT_ID'][3:] + 'C' + other = 'NAC' if camera == 'WAC' else 'WAC' + m0 = baseline.cmatrix() + + # The true, SPICE-derived inter-camera rotation + baseline_rel = (Frame.as_wayframe('CASSINI_ISS_' + other) + .wrt(Frame.as_wayframe('CASSINI_ISS_' + camera)) + .transform_at_time(baseline.tstart).matrix) + + # First override the label camera alone with an unrelated custom + # pointing. This replaces the global CASSINI_ISS_ frame. + bogus = _PERTURBATION * _PERTURBATION * m0 + _ = from_file(self.filespec, cmatrix=bogus) + + # Now load with mapping. rel must come from the dedicated *_SPICE + # frames, unaffected by the override above. + custom = _PERTURBATION * m0 + obs = from_file(self.filespec, cmatrix=custom, map_other_camera=True) + + self.assertTrue(np.all(obs.cmatrix().vals == custom.vals)) + + other_matrix = (Frame.as_wayframe('CASSINI_ISS_' + other) + .wrt(Frame.J2000) + .transform_at_time(baseline.tstart).matrix) + self.assertTrue(np.allclose(other_matrix.vals, + (baseline_rel * custom).vals)) + + #=========================================================================== + def test_no_ck_loaded_without_mapping(self): + """A custom cmatrix without map_other_camera never loads a CK (no + SPICE pointing dependency).""" + + self.assertFalse(np.any(Cassini.CK_LOADED)) + + _ = from_file(self.filespec, cmatrix=oops.Matrix3(np.eye(3))) + + self.assertFalse(np.any(Cassini.CK_LOADED)) + + #=========================================================================== + def test_mapping_or_spice_pointing_loads_ck(self): + """SPICE pointing (no cmatrix), and map_other_camera=True, each load + a CK.""" + + self.assertFalse(np.any(Cassini.CK_LOADED)) + _ = from_file(self.filespec) + self.assertTrue(np.any(Cassini.CK_LOADED)) + + ISS.reset() + self.assertFalse(np.any(Cassini.CK_LOADED)) + _ = from_file(self.filespec, cmatrix=oops.Matrix3(np.eye(3)), + map_other_camera=True) + self.assertTrue(np.any(Cassini.CK_LOADED)) + + #=========================================================================== + def test_reset_restores_spice_pointing(self): + """After ISS.reset(), from_file() without cmatrix restores the + SPICE-derived pointing even if a custom cmatrix was used earlier.""" + + baseline = from_file(self.filespec) + m0 = baseline.cmatrix() + + custom = _PERTURBATION * m0 + _ = from_file(self.filespec, cmatrix=custom) + + ISS.reset() + restored = from_file(self.filespec) + self.assertTrue(np.allclose(restored.cmatrix().vals, m0.vals)) + +############################################ +if __name__ == '__main__': + unittest.main(verbosity=2) +################################################################################ diff --git a/tests/hosts/cassini/iss/unittester.py b/tests/hosts/cassini/iss/unittester.py index ee0c0c39..54bad579 100644 --- a/tests/hosts/cassini/iss/unittester.py +++ b/tests/hosts/cassini/iss/unittester.py @@ -5,6 +5,7 @@ import unittest from tests.hosts.cassini.iss import Test_Cassini_ISS_GoldMaster +from tests.hosts.cassini.iss.test_iss import Test_Cassini_ISS_Cmatrix ######################################## if __name__ == '__main__': diff --git a/tests/observation/test_snapshot.py b/tests/observation/test_snapshot.py index 8d821a81..e10e3275 100755 --- a/tests/observation/test_snapshot.py +++ b/tests/observation/test_snapshot.py @@ -5,9 +5,9 @@ import numpy as np import unittest -from polymath import Matrix3, Pair, Vector +from polymath import Matrix3, Pair, Scalar, Vector from oops.fov import FlatFOV -from oops.frame import Cmatrix +from oops.frame import Cmatrix, SpinFrame from oops.observation import Snapshot @@ -138,6 +138,45 @@ def runTest(self): self.assertTrue(np.all(obs.cmatrix(reference=obs.frame).vals == np.eye(3))) + # cmatrix() with a time-dependent frame, using both uv and time + # + # The Cmatrix frame above is fixed in time, so it cannot reveal whether + # cmatrix() actually consults uv and time. Here we combine a rotating + # SpinFrame (a different matrix at every time) with an observation whose + # midtime_at_uv() varies with the u coordinate, so distinct UVs must map + # to distinct times and therefore distinct matrices. + + spin = SpinFrame(0., 1., 0., 2, 'J2000', + frame_id='TEST_SNAPSHOT_SPIN') + + def spin_matrix(t): + # The J2000 -> SpinFrame rotation at time t (see SpinFrame, axis=2) + (c, s) = (np.cos(t), np.sin(t)) + return np.array([[c, s, 0.], [-s, c, 0.], [0., 0., 1.]]) + + class TimeDependentSnapshot(Snapshot): + # midtime depends on the u coordinate: midtime == u + def midtime_at_uv(self, uv, tfrac=0.5): + return Scalar.as_scalar(Pair.as_pair(uv).to_scalar(0)) + + obs = TimeDependentSnapshot(('u','v'), tstart=98., texp=2., + fov=fov, path='SSB', frame=spin) + + # Selecting different UVs selects different times -> different matrices + cmat1 = obs.cmatrix(uv=(1.,4.)) + cmat2 = obs.cmatrix(uv=(2.,4.)) + self.assertTrue(np.allclose(cmat1.vals, spin_matrix(1.))) + self.assertTrue(np.allclose(cmat2.vals, spin_matrix(2.))) + self.assertFalse(np.allclose(cmat1.vals, cmat2.vals)) + + # The default uv is the center of the FOV (uv_shape/2 -> u == 5) + self.assertTrue(np.allclose(obs.cmatrix().vals, spin_matrix(5.))) + + # An explicit time is used directly, overriding uv + self.assertTrue(np.allclose(obs.cmatrix(time=3.).vals, spin_matrix(3.))) + self.assertTrue(np.allclose(obs.cmatrix(uv=(1.,4.), time=3.).vals, + spin_matrix(3.))) + ################################################################################ if __name__ == '__main__': unittest.main(verbosity=2) From d92a221361f75fb1255db839aa82bb17be0f0313 Mon Sep 17 00:00:00 2001 From: Joseph Spitale Date: Tue, 11 Aug 2026 12:13:14 -0700 Subject: [PATCH 04/22] Fix custom C-matrix conventions and frame leaks in Cassini ISS A recorded C-matrix is the SPICE camera-frame attitude, but the oops observation frame is that frame rotated 180 deg about the boresight, so feeding one into from_file(cmatrix=...) landed the frame ~0.5 deg off. Apply the fixed rotation (ROT180) at the boundary in ISS.set_cmatrix so a recorded/pxform C-matrix reproduces the observation's pointing, and add ISS.oops_from_host / host_from_oops to convert between the two conventions. Also fix two frame-registry defects: - A custom C-matrix no longer leaks into the next plain load. The default (frame_id=None) case now gives each observation its own unregistered Cmatrix frame instead of overriding the global CASSINI_ISS_, so loading another image never changes an earlier one's pointing. The map_other_camera case, which does override the globals, now clears ISS.frames_defined so the next plain load rebuilds them from SPICE. Correct the Observation.cmatrix() docstring: it returns the generic observation-frame attitude, which for ISS is not interchangeable with the SPICE-convention cmatrix input. Extend the ISS cmatrix tests: a pxform round-trip (zero offset), the default-frame isolation and no-leak regressions, the map reclaim path, and the oops_from_host / host_from_oops conversions. Co-Authored-By: Claude Opus 4.8 --- oops/hosts/cassini/iss.py | 136 +++++++++++++++----- oops/observation/observation_.py | 11 +- tests/hosts/cassini/iss/test_iss.py | 190 ++++++++++++++++++++++------ 3 files changed, 260 insertions(+), 77 deletions(-) diff --git a/oops/hosts/cassini/iss.py b/oops/hosts/cassini/iss.py index 530d4a65..bff1c88b 100755 --- a/oops/hosts/cassini/iss.py +++ b/oops/hosts/cassini/iss.py @@ -13,6 +13,14 @@ import oops from oops.hosts.cassini import Cassini +# The ISS observation frame is deliberately the SPICE camera frame rotated 180 +# degrees about the boresight, because the instrument's internal coordinate +# system is flipped relative to the SPICE CASSINI_ISS_ frame. A recorded +# C-matrix (from a CK or cspyce.pxform) is in the *SPICE* camera +# frame, so this rotation is applied at the boundary to convert it into an oops +# observation frame. It is its own inverse. +ROT180 = oops.Matrix3([[-1,0,0],[0,-1,0],[0,0,1]]) + ################################################################################ # Standard class methods ################################################################################ @@ -33,14 +41,19 @@ def from_file(filespec, fast_distortion=True, return_all_planets Include kernels for all planets not just Jupiter or Saturn. - cmatrix an oops.Matrix3 (or 3x3 array) giving the rotation - from J2000 into the camera image frame, used instead - of SPICE pointing. None uses SPICE as before. + cmatrix an oops.Matrix3 (or 3x3 array) giving the SPICE + camera-frame C-matrix: the rotation from J2000 into + the SPICE CASSINI_ISS_ frame, exactly as + returned by cspyce.pxform() or recorded in a CK. The + fixed 180-degree instrument rotation relating that + frame to the oops observation frame is applied + internally. None uses SPICE pointing as before. frame_id when cmatrix is given and map_other_camera is False, register the C-matrix under this frame ID and use only - that frame; None overrides the global - CASSINI_ISS_ frame. + that (shared, registered) frame. None (default) gives + the observation its own unregistered frame, so loading + other images never disturbs this one's pointing. map_other_camera when True, also derive and register the other camera's frame from the fixed inter-camera rotation (uses @@ -91,16 +104,15 @@ def from_file(filespec, fast_distortion=True, Cassini.load_cks(tstart, tstart + texp) # Determine the observation frame, applying a custom C-matrix if given. - # ISS.set_cmatrix registers the frame(s); SPICE pointing is circumvented - # unless the other camera is being mapped. - frame = 'CASSINI_ISS_' + camera + # ISS.set_cmatrix builds the frame(s) and returns the one to attach to this + # observation; SPICE pointing is circumvented unless the other camera is + # being mapped. if cmatrix is not None: - ISS.set_cmatrix(cmatrix, camera, frame_id=frame_id, - map_other_camera=map_other_camera, time=tstart) - if frame_id is not None and not map_other_camera: - frame = frame_id + frame = ISS.set_cmatrix(cmatrix, camera, frame_id=frame_id, + map_other_camera=map_other_camera, time=tstart) else: ISS.define_camera_frames() # use the SPICE-derived pointing + frame = 'CASSINI_ISS_' + camera # Create a Snapshot result = oops.obs.Snapshot(('v','u'), tstart, texp, @@ -432,7 +444,7 @@ def define_camera_frames(): # Construct a SpiceFrame for each camera # Deal with the fact that the instrument's internal # coordinate system is rotated 180 degrees - rot180 = oops.Matrix3([[-1,0,0],[0,-1,0],[0,0,1]]) + rot180 = ROT180 nac_flipped = oops.frame.SpiceFrame('CASSINI_ISS_NAC', frame_id='CASSINI_ISS_NAC_FLIPPED') wac_flipped = oops.frame.SpiceFrame('CASSINI_ISS_WAC', @@ -491,31 +503,82 @@ def define_camera_frames(): ISS.frames_defined = True + #=========================================================================== + @staticmethod + def oops_from_host(cmatrix): + """Convert a host C-matrix into the oops observation-frame attitude. + + The host convention is the SPICE camera-frame C-matrix: the rotation + from J2000 into SPICE's CASSINI_ISS_ frame, as returned by + cspyce.pxform() or recorded in a CK. The oops convention is the attitude + of the oops observation frame (what Observation.cmatrix() returns), which + is that SPICE camera frame rotated 180 degrees about the boresight. + + Input: + cmatrix an oops.Matrix3 (or 3x3 array) in the host (SPICE + camera-frame) convention. + + Return: an oops.Matrix3 in the oops observation-frame convention. + """ + + return ROT180 * oops.Matrix3.as_matrix3(cmatrix) + + #=========================================================================== + @staticmethod + def host_from_oops(cmatrix): + """Convert an oops observation-frame attitude into a host C-matrix. + + The inverse of oops_from_host(); see that method for the two conventions. + Because the frames differ by a 180-degree rotation (its own inverse), + this applies the same rotation. + + Input: + cmatrix an oops.Matrix3 (or 3x3 array) in the oops + observation-frame convention (e.g. Observation.cmatrix()). + + Return: an oops.Matrix3 in the host (SPICE camera-frame) + convention. + """ + + return ROT180 * oops.Matrix3.as_matrix3(cmatrix) + #=========================================================================== @staticmethod def set_cmatrix(cmatrix, camera, frame_id=None, map_other_camera=False, time=None): - """Register a custom C-matrix as an ISS observation's pointing, replacing - the SPICE-derived camera frame. + """Build an oops observation frame from a custom SPICE camera-frame + C-matrix and return the frame to attach to the observation. - ISS.initialize() must have been called first. The C-matrix gives the - rotation from J2000 into the camera image frame (X right, Y down, Z along - the optic axis). + ISS.initialize() must have been called first. The C-matrix is the SPICE + camera-frame pointing (the rotation from J2000 into SPICE's + CASSINI_ISS_ frame, as returned by cspyce.pxform() or recorded in + a CK). The oops observation frame is that frame rotated 180 degrees about + the boresight (see ROT180); this rotation is applied here, at the + boundary, so a recorded C-matrix reproduces the observation's pointing. Input: - cmatrix an oops.Matrix3 (or 3x3 array) for the named camera. + cmatrix an oops.Matrix3 (or 3x3 array) for the named camera, in + the SPICE camera-frame convention. camera 'NAC' or 'WAC', the camera the cmatrix applies to. - frame_id when map_other_camera is False, register the C-matrix - under this frame ID; None overrides the global - CASSINI_ISS_ frame. + frame_id when map_other_camera is False: None (default) returns a + fresh unregistered frame owned by the observation, so + loading other images never disturbs its pointing; + otherwise the frame is registered under this frame ID (a + shared, registered frame). map_other_camera when True, also derive and register the other camera's frame from the fixed inter-camera rotation (uses SPICE); frame_id is ignored. time the observation time (TDB), required to evaluate the inter-camera rotation when map_other_camera is True. + + Return: the frame to attach to the observation: a Cmatrix frame + object (unregistered default case) or a registered frame + ID (frame_id given, or map_other_camera). """ - cmatrix = oops.Matrix3.as_matrix3(cmatrix) + # Convert the SPICE camera-frame C-matrix into the oops observation-frame + # attitude by applying the fixed 180-degree instrument rotation. + attitude = ISS.oops_from_host(cmatrix) if map_other_camera: # rel = M_other<-label, the fixed rotation read from the dedicated @@ -531,19 +594,28 @@ def set_cmatrix(cmatrix, camera, frame_id=None, map_other_camera=False, + '_SPICE') rel = other_wf.wrt(label_wf).transform_at_time(time).matrix - oops.frame.Cmatrix(cmatrix, frame_id='CASSINI_ISS_' + camera, + oops.frame.Cmatrix(attitude, frame_id='CASSINI_ISS_' + camera, override=True) - oops.frame.Cmatrix(rel * cmatrix, + oops.frame.Cmatrix(rel * attitude, frame_id='CASSINI_ISS_' + other, override=True) - elif frame_id is not None: - # Single custom frame; global frames untouched - oops.frame.Cmatrix(cmatrix, frame_id=frame_id) + # The global camera frames now hold this observation's custom + # pointing. Force define_camera_frames() to rebuild them from SPICE on + # the next plain load; otherwise its quick-exit would leak this + # pointing into the next image. + ISS.frames_defined = False - else: - # Override the label camera's global frame - oops.frame.Cmatrix(cmatrix, frame_id='CASSINI_ISS_' + camera, - override=True) + return 'CASSINI_ISS_' + camera + + if frame_id is not None: + # Register a single, shared custom frame; global frames untouched. + oops.frame.Cmatrix(attitude, frame_id=frame_id) + return frame_id + + # Default: a fresh unregistered frame owned by this observation. It never + # enters the global registry, so it neither collides with nor is + # overwritten by any other image's pointing. + return oops.frame.Cmatrix(attitude, frame_id=None) #=========================================================================== @staticmethod diff --git a/oops/observation/observation_.py b/oops/observation/observation_.py index ac4d3244..eea87963 100755 --- a/oops/observation/observation_.py +++ b/oops/observation/observation_.py @@ -456,10 +456,13 @@ def cmatrix(self, uv=None, time=None, reference=None): """The 3x3 rotation matrix that rotates a reference frame into this observation's frame at a selected time. - This is the same J2000 -> camera-frame rotation accepted as a - "cmatrix" input elsewhere in this package (e.g., - oops.hosts.cassini.iss.from_file(cmatrix=...)), so the result of this - method can be fed back in to reproduce this observation's pointing. + This returns the attitude of the observation frame itself (by default, + the J2000 -> observation-frame rotation). Note that a host's "cmatrix" + input may use a different convention: e.g. + oops.hosts.cassini.iss.from_file(cmatrix=...) takes the SPICE + camera-frame C-matrix, which for ISS differs from the observation frame + by a fixed 180-degree rotation about the boresight. The value returned + here is therefore not, in general, interchangeable with such an input. Input: uv a (u,v) pixel location, used only to select a time diff --git a/tests/hosts/cassini/iss/test_iss.py b/tests/hosts/cassini/iss/test_iss.py index ca553a41..9f482b7c 100644 --- a/tests/hosts/cassini/iss/test_iss.py +++ b/tests/hosts/cassini/iss/test_iss.py @@ -9,7 +9,7 @@ from oops.body import Body from oops.frame import Frame from oops.hosts.cassini import Cassini -from oops.hosts.cassini.iss import ISS, from_file +from oops.hosts.cassini.iss import ISS, from_file, ROT180 from oops.unittester_support import TEST_DATA_PREFIX # A rotation distinguishable from any SPICE-derived pointing, used to build @@ -21,12 +21,18 @@ class Test_Cassini_ISS_Cmatrix(unittest.TestCase): """Tests for the custom C-matrix support in hosts/cassini/iss.py (from_file's cmatrix/frame_id/map_other_camera arguments and ISS.set_cmatrix), plus the generic Observation.cmatrix() getter. + + The `cmatrix` accepted by from_file is the SPICE camera-frame C-matrix (the + J2000 -> CASSINI_ISS_ rotation, as returned by cspyce.pxform). The + oops observation frame is that frame rotated 180 degrees about the boresight, + so an observation's attitude (Observation.cmatrix()) equals ROT180 times the + SPICE C-matrix that produced it. """ FILESPEC = 'cassini/ISS/W1573721822_1.IMG' # a WAC image def setUp(self): - # Custom C-matrices can override the global CASSINI_ISS_NAC/WAC + # A mapped custom C-matrix can override the global CASSINI_ISS_NAC/WAC # frames, so every test starts from a clean registry. Body.reset_registry() Body.define_solar_system() @@ -40,42 +46,125 @@ def tearDown(self): ISS.reset() #=========================================================================== - def test_default_override(self): - """A custom cmatrix with no frame_id overrides the global camera - frame, and Observation.cmatrix() round-trips it.""" + def _spice_cmatrix(self, camera, time): + """The recorded SPICE camera-frame C-matrix for the given camera and + time. CASSINI_ISS__FLIPPED is the SpiceFrame wrapping SPICE's + CASSINI_ISS_, so its J2000 attitude is exactly + cspyce.pxform('J2000', 'CASSINI_ISS_', time). Requires a prior + plain load (define_camera_frames) so the *_FLIPPED frame is registered. + """ + + return (Frame.as_wayframe('CASSINI_ISS_' + camera + '_FLIPPED') + .wrt(Frame.J2000).transform_at_time(time).matrix) + + #=========================================================================== + def test_pxform_roundtrip(self): + """Feeding the recorded SPICE C-matrix (what pxform returns) straight + into from_file reproduces the SPICE pointing exactly, because the fixed + 180-degree instrument rotation is applied at the boundary.""" + + baseline = from_file(self.filespec) # SPICE pointing + camera = baseline.dict['INSTRUMENT_ID'][3:] + 'C' + spice_cmatrix = self._spice_cmatrix(camera, baseline.tstart) + + obs = from_file(self.filespec, cmatrix=spice_cmatrix) + + # Same pointing as the SPICE-derived observation: zero offset. + self.assertTrue(np.allclose(obs.cmatrix().vals, baseline.cmatrix().vals)) + # The boundary rotation really was applied: the observation-frame + # attitude is ROT180 times the raw SPICE C-matrix, not the C-matrix + # itself (the two differ by the full 180 degrees). + self.assertTrue(np.allclose(obs.cmatrix().vals, + (ROT180 * spice_cmatrix).vals)) + self.assertFalse(np.allclose(obs.cmatrix().vals, spice_cmatrix.vals)) + + #=========================================================================== + def test_convention_conversions(self): + """ISS.oops_from_host / host_from_oops convert between the two + conventions, are mutual inverses, and match the from_file / + Observation.cmatrix() endpoints.""" + + # Mutual inverses, for an arbitrary rotation. + m = _PERTURBATION * ROT180 * _PERTURBATION + self.assertTrue(np.allclose(m.vals, + ISS.host_from_oops(ISS.oops_from_host(m)).vals)) + self.assertTrue(np.allclose(m.vals, + ISS.oops_from_host(ISS.host_from_oops(m)).vals)) + + # They tie the SPICE C-matrix (pxform) to the oops observation-frame + # attitude used by from_file() and returned by Observation.cmatrix(). baseline = from_file(self.filespec) camera = baseline.dict['INSTRUMENT_ID'][3:] + 'C' - custom = _PERTURBATION * baseline.cmatrix() + spice_cmatrix = self._spice_cmatrix(camera, baseline.tstart) - obs = from_file(self.filespec, cmatrix=custom) + self.assertTrue(np.allclose(ISS.oops_from_host(spice_cmatrix).vals, + baseline.cmatrix().vals)) + self.assertTrue(np.allclose(ISS.host_from_oops(baseline.cmatrix()).vals, + spice_cmatrix.vals)) - self.assertEqual(obs.frame.frame_id, 'CASSINI_ISS_' + camera) - self.assertTrue(np.all(obs.cmatrix().vals == custom.vals)) + #=========================================================================== + def test_default_frame_is_unregistered_and_isolated(self): + """With frame_id=None each observation gets its own unregistered frame, + so loading another image never changes an earlier one's pointing.""" + + baseline = from_file(self.filespec) + camera = baseline.dict['INSTRUMENT_ID'][3:] + 'C' + spice0 = self._spice_cmatrix(camera, baseline.tstart) + + custom1 = _PERTURBATION * spice0 + obs1 = from_file(self.filespec, cmatrix=custom1) + saved1 = obs1.cmatrix() + + # The observation owns an unregistered frame, not the global wayframe. + self.assertFalse(obs1.frame.is_registered()) + self.assertTrue(np.allclose(obs1.cmatrix().vals, (ROT180 * custom1).vals)) + + # Loading a second custom image must not disturb the first. + custom2 = _PERTURBATION * _PERTURBATION * spice0 + obs2 = from_file(self.filespec, cmatrix=custom2) + + self.assertIsNot(obs1.frame, obs2.frame) + self.assertTrue(np.all(obs1.cmatrix().vals == saved1.vals)) + self.assertTrue(np.allclose(obs2.cmatrix().vals, (ROT180 * custom2).vals)) + self.assertFalse(np.allclose(obs1.cmatrix().vals, obs2.cmatrix().vals)) - # The global frame really was replaced, not merely shadowed - direct = (Frame.as_wayframe('CASSINI_ISS_' + camera) - .wrt(Frame.J2000).transform_at_time(obs.tstart).matrix) - self.assertTrue(np.all(direct.vals == custom.vals)) + #=========================================================================== + def test_default_does_not_leak_into_plain_load(self): + """A default custom load never touches the global camera frame, so a + subsequent plain load reads back its own SPICE pointing.""" + + baseline = from_file(self.filespec) + m0 = baseline.cmatrix() + camera = baseline.dict['INSTRUMENT_ID'][3:] + 'C' + spice0 = self._spice_cmatrix(camera, baseline.tstart) + + _ = from_file(self.filespec, cmatrix=_PERTURBATION * spice0) + + plain = from_file(self.filespec) + self.assertTrue(np.allclose(plain.cmatrix().vals, m0.vals)) #=========================================================================== def test_explicit_frame_id_leaves_global_untouched(self): """A custom cmatrix with an explicit frame_id registers a separate - frame and leaves the global CASSINI_ISS_ frame alone.""" + frame (with the boundary rotation applied) and leaves the global + CASSINI_ISS_ frame alone.""" baseline = from_file(self.filespec) + camera = baseline.dict['INSTRUMENT_ID'][3:] + 'C' m0 = baseline.cmatrix() - custom = _PERTURBATION * m0 + spice0 = self._spice_cmatrix(camera, baseline.tstart) + custom = _PERTURBATION * spice0 obs = from_file(self.filespec, cmatrix=custom, frame_id='TEST_ISS_CUSTOM_FRAME') self.assertEqual(obs.frame.frame_id, 'TEST_ISS_CUSTOM_FRAME') - self.assertTrue(np.all(obs.cmatrix().vals == custom.vals)) + self.assertTrue(np.allclose(obs.cmatrix().vals, (ROT180 * custom).vals)) - # The global camera frame is untouched + # The global camera frame is untouched. unaffected = from_file(self.filespec) - self.assertTrue(np.all(unaffected.cmatrix().vals == m0.vals)) + self.assertTrue(np.allclose(unaffected.cmatrix().vals, m0.vals)) #=========================================================================== def test_map_other_camera(self): @@ -86,64 +175,81 @@ def test_map_other_camera(self): baseline = from_file(self.filespec) camera = baseline.dict['INSTRUMENT_ID'][3:] + 'C' other = 'NAC' if camera == 'WAC' else 'WAC' - m0 = baseline.cmatrix() + spice0 = self._spice_cmatrix(camera, baseline.tstart) baseline_rel = (Frame.as_wayframe('CASSINI_ISS_' + other) .wrt(Frame.as_wayframe('CASSINI_ISS_' + camera)) .transform_at_time(baseline.tstart).matrix) - custom = _PERTURBATION * m0 + custom = _PERTURBATION * spice0 obs = from_file(self.filespec, cmatrix=custom, map_other_camera=True) self.assertEqual(obs.frame.frame_id, 'CASSINI_ISS_' + camera) - self.assertTrue(np.all(obs.cmatrix().vals == custom.vals)) + self.assertTrue(np.allclose(obs.cmatrix().vals, (ROT180 * custom).vals)) - # The fixed inter-camera rotation is preserved under the override + # The fixed inter-camera rotation is preserved under the override. new_rel = (Frame.as_wayframe('CASSINI_ISS_' + other) .wrt(Frame.as_wayframe('CASSINI_ISS_' + camera)) .transform_at_time(baseline.tstart).matrix) self.assertTrue(np.allclose(new_rel.vals, baseline_rel.vals)) - # ...and the other camera's absolute pointing was updated accordingly + # ...and the other camera's absolute pointing was updated accordingly. other_matrix = (Frame.as_wayframe('CASSINI_ISS_' + other) .wrt(Frame.J2000) .transform_at_time(baseline.tstart).matrix) self.assertTrue(np.allclose(other_matrix.vals, - (baseline_rel * custom).vals)) + (baseline_rel * ROT180 * custom).vals)) #=========================================================================== - def test_map_after_single_camera_override(self): - """A single-camera override followed by a mapped-camera load must still - derive the inter-camera rotation from SPICE, not from the overridden - CASSINI_ISS_ frame.""" + def test_map_reclaims_global_on_plain_load(self): + """A mapped custom load overrides the global camera frames; a later + plain load must rebuild them from SPICE rather than inheriting the + custom pointing.""" + + baseline = from_file(self.filespec) + m0 = baseline.cmatrix() + camera = baseline.dict['INSTRUMENT_ID'][3:] + 'C' + spice0 = self._spice_cmatrix(camera, baseline.tstart) + + _ = from_file(self.filespec, cmatrix=_PERTURBATION * spice0, + map_other_camera=True) + + plain = from_file(self.filespec) + self.assertTrue(np.allclose(plain.cmatrix().vals, m0.vals)) + + #=========================================================================== + def test_map_after_map_override(self): + """A mapped load followed by another mapped load must still derive the + inter-camera rotation from the dedicated *_SPICE frames, not from the + CASSINI_ISS_ frames a prior mapped load overrode.""" baseline = from_file(self.filespec) camera = baseline.dict['INSTRUMENT_ID'][3:] + 'C' other = 'NAC' if camera == 'WAC' else 'WAC' - m0 = baseline.cmatrix() + spice0 = self._spice_cmatrix(camera, baseline.tstart) - # The true, SPICE-derived inter-camera rotation + # The true, SPICE-derived inter-camera rotation. baseline_rel = (Frame.as_wayframe('CASSINI_ISS_' + other) .wrt(Frame.as_wayframe('CASSINI_ISS_' + camera)) .transform_at_time(baseline.tstart).matrix) - # First override the label camera alone with an unrelated custom - # pointing. This replaces the global CASSINI_ISS_ frame. - bogus = _PERTURBATION * _PERTURBATION * m0 - _ = from_file(self.filespec, cmatrix=bogus) + # First mapped load with an unrelated pointing overrides both global + # camera frames. + bogus = _PERTURBATION * _PERTURBATION * spice0 + _ = from_file(self.filespec, cmatrix=bogus, map_other_camera=True) - # Now load with mapping. rel must come from the dedicated *_SPICE - # frames, unaffected by the override above. - custom = _PERTURBATION * m0 + # Second mapped load: rel must come from the dedicated *_SPICE frames, + # unaffected by the override above. + custom = _PERTURBATION * spice0 obs = from_file(self.filespec, cmatrix=custom, map_other_camera=True) - self.assertTrue(np.all(obs.cmatrix().vals == custom.vals)) + self.assertTrue(np.allclose(obs.cmatrix().vals, (ROT180 * custom).vals)) other_matrix = (Frame.as_wayframe('CASSINI_ISS_' + other) .wrt(Frame.J2000) .transform_at_time(baseline.tstart).matrix) self.assertTrue(np.allclose(other_matrix.vals, - (baseline_rel * custom).vals)) + (baseline_rel * ROT180 * custom).vals)) #=========================================================================== def test_no_ck_loaded_without_mapping(self): @@ -178,9 +284,11 @@ def test_reset_restores_spice_pointing(self): baseline = from_file(self.filespec) m0 = baseline.cmatrix() + camera = baseline.dict['INSTRUMENT_ID'][3:] + 'C' + spice0 = self._spice_cmatrix(camera, baseline.tstart) - custom = _PERTURBATION * m0 - _ = from_file(self.filespec, cmatrix=custom) + _ = from_file(self.filespec, cmatrix=_PERTURBATION * spice0, + map_other_camera=True) ISS.reset() restored = from_file(self.filespec) From fc5219121b3ae64385df328715a25e8c62cef14d Mon Sep 17 00:00:00 2001 From: Joseph Spitale Date: Tue, 11 Aug 2026 12:26:07 -0700 Subject: [PATCH 05/22] Add ISS.get_cmatrix, the inverse of set_cmatrix Returns an ISS observation's pointing as a SPICE camera-frame C-matrix (as cspyce.pxform would return it), so it can be fed back into from_file(cmatrix=...) to reproduce the pointing. Implemented as host_from_oops(obs.cmatrix()), with uv/time selection delegated to Observation.cmatrix(). Adds a test covering the pxform match and the set/get round-trip. Co-Authored-By: Claude Opus 4.8 --- oops/hosts/cassini/iss.py | 24 ++++++++++++++++++++++++ tests/hosts/cassini/iss/test_iss.py | 24 ++++++++++++++++++++++++ 2 files changed, 48 insertions(+) diff --git a/oops/hosts/cassini/iss.py b/oops/hosts/cassini/iss.py index bff1c88b..a75b3734 100755 --- a/oops/hosts/cassini/iss.py +++ b/oops/hosts/cassini/iss.py @@ -617,6 +617,30 @@ def set_cmatrix(cmatrix, camera, frame_id=None, map_other_camera=False, # overwritten by any other image's pointing. return oops.frame.Cmatrix(attitude, frame_id=None) + #=========================================================================== + @staticmethod + def get_cmatrix(obs, uv=None, time=None): + """The SPICE camera-frame C-matrix of an ISS observation. + + The inverse of set_cmatrix(): it returns the observation's pointing in + the SPICE camera-frame convention (the rotation from J2000 into SPICE's + CASSINI_ISS_ frame, as cspyce.pxform() would return), so feeding + the result back into from_file(cmatrix=...) reproduces the pointing. + + Input: + obs an ISS observation (e.g. a Snapshot from from_file); its + frame must be the oops ISS observation frame. + uv a (u,v) pixel location, used only to select a time when + the observation frame is time-dependent; None to use the + center of the FOV. Ignored if time is given. + time the time in seconds TDB at which to evaluate the frame; + None to derive it from uv. + + Return: an oops.Matrix3, the SPICE camera-frame C-matrix. + """ + + return ISS.host_from_oops(obs.cmatrix(uv=uv, time=time)) + #=========================================================================== @staticmethod def reset(): diff --git a/tests/hosts/cassini/iss/test_iss.py b/tests/hosts/cassini/iss/test_iss.py index 9f482b7c..cbd1b6ad 100644 --- a/tests/hosts/cassini/iss/test_iss.py +++ b/tests/hosts/cassini/iss/test_iss.py @@ -103,6 +103,30 @@ def test_convention_conversions(self): self.assertTrue(np.allclose(ISS.host_from_oops(baseline.cmatrix()).vals, spice_cmatrix.vals)) + #=========================================================================== + def test_get_cmatrix_inverts_set(self): + """ISS.get_cmatrix returns the SPICE-convention C-matrix (matching + pxform) and inverts the cmatrix given to from_file.""" + + baseline = from_file(self.filespec) + camera = baseline.dict['INSTRUMENT_ID'][3:] + 'C' + spice0 = self._spice_cmatrix(camera, baseline.tstart) + + # For a SPICE-loaded observation, get_cmatrix matches pxform. + self.assertTrue(np.allclose( + ISS.get_cmatrix(baseline, time=baseline.tstart).vals, spice0.vals)) + + # It recovers the exact cmatrix given to from_file (the custom Snapshot + # frame is fixed in time, so no time argument is needed). + custom = _PERTURBATION * spice0 + obs = from_file(self.filespec, cmatrix=custom) + recovered = ISS.get_cmatrix(obs) + self.assertTrue(np.allclose(recovered.vals, custom.vals)) + + # Round-trip: feeding it back into from_file reproduces the pointing. + obs2 = from_file(self.filespec, cmatrix=recovered) + self.assertTrue(np.allclose(obs2.cmatrix().vals, obs.cmatrix().vals)) + #=========================================================================== def test_default_frame_is_unregistered_and_isolated(self): """With frame_id=None each observation gets its own unregistered frame, From 5ef6ce7aa42ca8e4e554be61bc282aac30044e1f Mon Sep 17 00:00:00 2001 From: Joseph Spitale Date: Tue, 11 Aug 2026 13:19:15 -0700 Subject: [PATCH 06/22] Use the ROT180 constant directly in define_camera_frames Drop the redundant rot180 = ROT180 local alias and reference the module-level ROT180 constant directly in the camera-frame constructions. Co-Authored-By: Claude Opus 4.8 --- oops/hosts/cassini/iss.py | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/oops/hosts/cassini/iss.py b/oops/hosts/cassini/iss.py index a75b3734..fb7c553f 100755 --- a/oops/hosts/cassini/iss.py +++ b/oops/hosts/cassini/iss.py @@ -443,8 +443,7 @@ def define_camera_frames(): # Construct a SpiceFrame for each camera # Deal with the fact that the instrument's internal - # coordinate system is rotated 180 degrees - rot180 = ROT180 + # coordinate system is rotated 180 degrees (see ROT180) nac_flipped = oops.frame.SpiceFrame('CASSINI_ISS_NAC', frame_id='CASSINI_ISS_NAC_FLIPPED') wac_flipped = oops.frame.SpiceFrame('CASSINI_ISS_WAC', @@ -471,34 +470,34 @@ def define_camera_frames(): # frames are never replaced by a custom C-matrix and remain a reliable # reference for the inter-camera rotation (see set_cmatrix). override=True # so a rebuild after ISS.reset() refreshes them to the current pointing. - oops.frame.Cmatrix(rot180, nac_flipped, + oops.frame.Cmatrix(ROT180, nac_flipped, frame_id='CASSINI_ISS_NAC_SPICE', override=True) if ISS.offset_wac: - wac_spice_no = oops.frame.Cmatrix(rot180, wac_flipped, + wac_spice_no = oops.frame.Cmatrix(ROT180, wac_flipped, frame_id='CASSINI_ISS_WAC-NO_OFFSET_SPICE', override=True) oops.frame.Navigation((xshift,yshift), wac_spice_no, frame_id='CASSINI_ISS_WAC_SPICE', override=True) else: - oops.frame.Cmatrix(rot180, wac_flipped, + oops.frame.Cmatrix(ROT180, wac_flipped, frame_id='CASSINI_ISS_WAC_SPICE', override=True) # Observation-facing camera frames. These start out identical to the # *_SPICE frames above, but set_cmatrix may later replace them with a # custom C-matrix. override=True: a prior custom C-matrix may already # have replaced the primary definition of this frame ID; reclaim it. - nac_frame = oops.frame.Cmatrix(rot180, nac_flipped, + nac_frame = oops.frame.Cmatrix(ROT180, nac_flipped, frame_id='CASSINI_ISS_NAC', override=True) if ISS.offset_wac: - wac_frame_no = oops.frame.Cmatrix(rot180, wac_flipped, + wac_frame_no = oops.frame.Cmatrix(ROT180, wac_flipped, frame_id='CASSINI_ISS_WAC-NO_OFFSET', override=True) wac_frame = oops.frame.Navigation((xshift,yshift), wac_frame_no, frame_id='CASSINI_ISS_WAC', override=True) else: - wac_frame = oops.frame.Cmatrix(rot180, wac_flipped, + wac_frame = oops.frame.Cmatrix(ROT180, wac_flipped, frame_id='CASSINI_ISS_WAC', override=True) ISS.frames_defined = True From c31fc066b353e470f68a8736854d98b7ac10c3bc Mon Sep 17 00:00:00 2001 From: Joseph Spitale Date: Tue, 11 Aug 2026 13:53:14 -0700 Subject: [PATCH 07/22] Extract set_cmatrix_both_cameras from set_cmatrix Move the map_other_camera branch of set_cmatrix into its own method, ISS.set_cmatrix_both_cameras(cmatrix, camera, time), which registers both camera frames from one camera's custom C-matrix and returns the label camera's frame ID. set_cmatrix now delegates to it. Behavior is unchanged. Co-Authored-By: Claude Opus 4.8 --- oops/hosts/cassini/iss.py | 84 ++++++++++++++++++++++++++------------- 1 file changed, 57 insertions(+), 27 deletions(-) diff --git a/oops/hosts/cassini/iss.py b/oops/hosts/cassini/iss.py index fb7c553f..95ecff7e 100755 --- a/oops/hosts/cassini/iss.py +++ b/oops/hosts/cassini/iss.py @@ -575,37 +575,13 @@ def set_cmatrix(cmatrix, camera, frame_id=None, map_other_camera=False, ID (frame_id given, or map_other_camera). """ + if map_other_camera: + return ISS.set_cmatrix_both_cameras(cmatrix, camera, time) + # Convert the SPICE camera-frame C-matrix into the oops observation-frame # attitude by applying the fixed 180-degree instrument rotation. attitude = ISS.oops_from_host(cmatrix) - if map_other_camera: - # rel = M_other<-label, the fixed rotation read from the dedicated - # *_SPICE frames at the observation time. These are used (rather than - # CASSINI_ISS_NAC/WAC) because a prior set_cmatrix override may have - # replaced the CASSINI_ISS_ frames with a custom C-matrix, - # which would corrupt the inter-camera rotation. - ISS.define_camera_frames() - other = 'WAC' if camera == 'NAC' else 'NAC' - label_wf = oops.frame.Frame.as_wayframe('CASSINI_ISS_' + camera - + '_SPICE') - other_wf = oops.frame.Frame.as_wayframe('CASSINI_ISS_' + other - + '_SPICE') - rel = other_wf.wrt(label_wf).transform_at_time(time).matrix - - oops.frame.Cmatrix(attitude, frame_id='CASSINI_ISS_' + camera, - override=True) - oops.frame.Cmatrix(rel * attitude, - frame_id='CASSINI_ISS_' + other, override=True) - - # The global camera frames now hold this observation's custom - # pointing. Force define_camera_frames() to rebuild them from SPICE on - # the next plain load; otherwise its quick-exit would leak this - # pointing into the next image. - ISS.frames_defined = False - - return 'CASSINI_ISS_' + camera - if frame_id is not None: # Register a single, shared custom frame; global frames untouched. oops.frame.Cmatrix(attitude, frame_id=frame_id) @@ -616,6 +592,60 @@ def set_cmatrix(cmatrix, camera, frame_id=None, map_other_camera=False, # overwritten by any other image's pointing. return oops.frame.Cmatrix(attitude, frame_id=None) + #=========================================================================== + @staticmethod + def set_cmatrix_both_cameras(cmatrix, camera, time): + """Register both CASSINI_ISS_NAC and CASSINI_ISS_WAC from one camera's + custom C-matrix, and return the label camera's frame ID. + + The other camera's frame is derived from the fixed, SPICE-derived + inter-camera rotation, so the two co-mounted cameras stay consistent + under the custom pointing. Unlike the single-camera path in + set_cmatrix(), this overrides the global camera frames (it is the + map_other_camera=True case of set_cmatrix()). + + ISS.initialize() must have been called first. + + Input: + cmatrix an oops.Matrix3 (or 3x3 array) for the named camera, in + the SPICE camera-frame convention (see set_cmatrix()). + camera 'NAC' or 'WAC', the camera the cmatrix applies to. + time the observation time (TDB), used to evaluate the + inter-camera rotation. + + Return: 'CASSINI_ISS_', the label camera's frame ID. + """ + + # Convert the SPICE camera-frame C-matrix into the oops observation-frame + # attitude by applying the fixed 180-degree instrument rotation. + attitude = ISS.oops_from_host(cmatrix) + + # rel = M_other<-label, the fixed rotation read from the dedicated + # *_SPICE frames at the observation time. These are used (rather than + # CASSINI_ISS_NAC/WAC) because a prior set_cmatrix override may have + # replaced the CASSINI_ISS_ frames with a custom C-matrix, + # which would corrupt the inter-camera rotation. + ISS.define_camera_frames() + other = 'WAC' if camera == 'NAC' else 'NAC' + label_wf = oops.frame.Frame.as_wayframe('CASSINI_ISS_' + camera + + '_SPICE') + other_wf = oops.frame.Frame.as_wayframe('CASSINI_ISS_' + other + + '_SPICE') + rel = other_wf.wrt(label_wf).transform_at_time(time).matrix + + oops.frame.Cmatrix(attitude, frame_id='CASSINI_ISS_' + camera, + override=True) + oops.frame.Cmatrix(rel * attitude, + frame_id='CASSINI_ISS_' + other, override=True) + + # The global camera frames now hold this observation's custom pointing. + # Force define_camera_frames() to rebuild them from SPICE on the next + # plain load; otherwise its quick-exit would leak this pointing into the + # next image. + ISS.frames_defined = False + + return 'CASSINI_ISS_' + camera + #=========================================================================== @staticmethod def get_cmatrix(obs, uv=None, time=None): From 43a87493faecd2ffa1254028cedbdace6633a83b Mon Sep 17 00:00:00 2001 From: Joseph Spitale Date: Tue, 11 Aug 2026 14:01:40 -0700 Subject: [PATCH 08/22] Rename set_cmatrix_both_cameras to map_other_camera Co-Authored-By: Claude Opus 4.8 --- oops/hosts/cassini/iss.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/oops/hosts/cassini/iss.py b/oops/hosts/cassini/iss.py index 95ecff7e..52361427 100755 --- a/oops/hosts/cassini/iss.py +++ b/oops/hosts/cassini/iss.py @@ -576,7 +576,7 @@ def set_cmatrix(cmatrix, camera, frame_id=None, map_other_camera=False, """ if map_other_camera: - return ISS.set_cmatrix_both_cameras(cmatrix, camera, time) + return ISS.map_other_camera(cmatrix, camera, time) # Convert the SPICE camera-frame C-matrix into the oops observation-frame # attitude by applying the fixed 180-degree instrument rotation. @@ -594,7 +594,7 @@ def set_cmatrix(cmatrix, camera, frame_id=None, map_other_camera=False, #=========================================================================== @staticmethod - def set_cmatrix_both_cameras(cmatrix, camera, time): + def map_other_camera(cmatrix, camera, time): """Register both CASSINI_ISS_NAC and CASSINI_ISS_WAC from one camera's custom C-matrix, and return the label camera's frame ID. From 9ef57d53443f7fed91da3199360ff57cf77c178d Mon Sep 17 00:00:00 2001 From: Joseph Spitale Date: Tue, 11 Aug 2026 14:07:43 -0700 Subject: [PATCH 09/22] Map the current camera's pointing in map_other_camera Remove the cmatrix argument from ISS.map_other_camera; it now reads the label camera's current pointing from the registered CASSINI_ISS_ frame and maps that to the other camera via the fixed inter-camera rotation. set_cmatrix's map_other_camera=True branch now installs the label camera's custom frame before delegating. Behavior is unchanged. Co-Authored-By: Claude Opus 4.8 --- oops/hosts/cassini/iss.py | 58 +++++++++++++++++++++------------------ 1 file changed, 31 insertions(+), 27 deletions(-) diff --git a/oops/hosts/cassini/iss.py b/oops/hosts/cassini/iss.py index 52361427..45d1364b 100755 --- a/oops/hosts/cassini/iss.py +++ b/oops/hosts/cassini/iss.py @@ -575,13 +575,20 @@ def set_cmatrix(cmatrix, camera, frame_id=None, map_other_camera=False, ID (frame_id given, or map_other_camera). """ - if map_other_camera: - return ISS.map_other_camera(cmatrix, camera, time) - # Convert the SPICE camera-frame C-matrix into the oops observation-frame # attitude by applying the fixed 180-degree instrument rotation. attitude = ISS.oops_from_host(cmatrix) + if map_other_camera: + # Build the SPICE-derived frames first (so the dedicated *_SPICE + # frames exist and the label camera starts from SPICE), then override + # the label camera's global frame and map that pointing to the other + # camera. + ISS.define_camera_frames() + oops.frame.Cmatrix(attitude, frame_id='CASSINI_ISS_' + camera, + override=True) + return ISS.map_other_camera(camera, time) + if frame_id is not None: # Register a single, shared custom frame; global frames untouched. oops.frame.Cmatrix(attitude, frame_id=frame_id) @@ -594,38 +601,33 @@ def set_cmatrix(cmatrix, camera, frame_id=None, map_other_camera=False, #=========================================================================== @staticmethod - def map_other_camera(cmatrix, camera, time): - """Register both CASSINI_ISS_NAC and CASSINI_ISS_WAC from one camera's - custom C-matrix, and return the label camera's frame ID. + def map_other_camera(camera, time): + """Register the other camera's frame from the label camera's current + pointing and the fixed inter-camera rotation, and return the label + camera's frame ID. - The other camera's frame is derived from the fixed, SPICE-derived - inter-camera rotation, so the two co-mounted cameras stay consistent - under the custom pointing. Unlike the single-camera path in - set_cmatrix(), this overrides the global camera frames (it is the - map_other_camera=True case of set_cmatrix()). + Takes whatever pointing the label camera's global frame currently holds + (SPICE-derived, or a custom C-matrix already installed by set_cmatrix) + and maps it to the co-mounted camera, so the two stay consistent. This + overrides the global camera frames. - ISS.initialize() must have been called first. + The label camera's global frame (CASSINI_ISS_) and the dedicated + *_SPICE frames must already be defined; ISS.define_camera_frames() (or + set_cmatrix() with map_other_camera=True) does this. Input: - cmatrix an oops.Matrix3 (or 3x3 array) for the named camera, in - the SPICE camera-frame convention (see set_cmatrix()). - camera 'NAC' or 'WAC', the camera the cmatrix applies to. + camera 'NAC' or 'WAC', the label camera whose pointing is mapped. time the observation time (TDB), used to evaluate the - inter-camera rotation. + inter-camera rotation and read the label pointing. Return: 'CASSINI_ISS_', the label camera's frame ID. """ - # Convert the SPICE camera-frame C-matrix into the oops observation-frame - # attitude by applying the fixed 180-degree instrument rotation. - attitude = ISS.oops_from_host(cmatrix) - # rel = M_other<-label, the fixed rotation read from the dedicated # *_SPICE frames at the observation time. These are used (rather than - # CASSINI_ISS_NAC/WAC) because a prior set_cmatrix override may have - # replaced the CASSINI_ISS_ frames with a custom C-matrix, - # which would corrupt the inter-camera rotation. - ISS.define_camera_frames() + # CASSINI_ISS_NAC/WAC) because a set_cmatrix override may have replaced + # the CASSINI_ISS_ frames with a custom C-matrix, which would + # corrupt the inter-camera rotation. other = 'WAC' if camera == 'NAC' else 'NAC' label_wf = oops.frame.Frame.as_wayframe('CASSINI_ISS_' + camera + '_SPICE') @@ -633,9 +635,11 @@ def map_other_camera(cmatrix, camera, time): + '_SPICE') rel = other_wf.wrt(label_wf).transform_at_time(time).matrix - oops.frame.Cmatrix(attitude, frame_id='CASSINI_ISS_' + camera, - override=True) - oops.frame.Cmatrix(rel * attitude, + # The label camera's current pointing (SPICE or a custom C-matrix). + label = oops.frame.Frame.as_wayframe('CASSINI_ISS_' + camera) \ + .wrt(oops.frame.Frame.J2000).transform_at_time(time).matrix + + oops.frame.Cmatrix(rel * label, frame_id='CASSINI_ISS_' + other, override=True) # The global camera frames now hold this observation's custom pointing. From 31ca78a93e52af4438bf99a8a7baf4490de82a56 Mon Sep 17 00:00:00 2001 From: Joseph Spitale Date: Wed, 12 Aug 2026 15:10:07 -0700 Subject: [PATCH 10/22] Derive the spice-frame C-matrix in get_cmatrix from the observation Observation.get_cmatrix() now derives the spice-frame (SPICE camera-frame) C-matrix at call time as host.CMATRIX_ROTATION * self.frame.wrt(J2000), with no stored cmatrix_frame subfield. The observation carries only a `host` reference; ISS exposes CMATRIX_ROTATION (renamed from ROT180) as a class attribute for this. - Remove the per-observation cmatrix_frame subfield from from_file/from_index. - Remove the redundant Observation.cmatrix() (raw oops-frame attitude accessor) and the host-side ISS.get_cmatrix(); get_cmatrix() is now the sole accessor. - Rename conversion helpers host_from_oops/oops_from_host -> spice_from_oops/oops_from_spice. - Rename ROT180 -> CMATRIX_ROTATION throughout. - Clarify docstrings around the two conventions: spice-frame (z=LOS, x left, y up) vs oops-frame (z=LOS, x right, y down), related by CMATRIX_ROTATION. - Use dedicated *_SPICE frames in define_camera_frames. - Repoint tests to get_cmatrix(); drop the removed cmatrix() tests. Co-Authored-By: Claude Opus 4.8 --- oops/hosts/cassini/iss.py | 154 +++++++++++++--------------- oops/observation/observation_.py | 50 +++++---- tests/hosts/cassini/iss/test_iss.py | 103 ++++++------------- tests/observation/test_snapshot.py | 55 +--------- 4 files changed, 130 insertions(+), 232 deletions(-) diff --git a/oops/hosts/cassini/iss.py b/oops/hosts/cassini/iss.py index 45d1364b..e8254752 100755 --- a/oops/hosts/cassini/iss.py +++ b/oops/hosts/cassini/iss.py @@ -13,13 +13,17 @@ import oops from oops.hosts.cassini import Cassini -# The ISS observation frame is deliberately the SPICE camera frame rotated 180 -# degrees about the boresight, because the instrument's internal coordinate -# system is flipped relative to the SPICE CASSINI_ISS_ frame. A recorded -# C-matrix (from a CK or cspyce.pxform) is in the *SPICE* camera -# frame, so this rotation is applied at the boundary to convert it into an oops -# observation frame. It is its own inverse. -ROT180 = oops.Matrix3([[-1,0,0],[0,-1,0],[0,0,1]]) +# There are two C-matrix conventions here, related by CMATRIX_ROTATION (a 180-degree spin +# about the boresight; it is its own inverse): +# * spice-frame: the pointing straight from SPICE (a CK or cspyce.pxform), i.e. +# the J2000 -> CASSINI_ISS_ rotation. Axes: z along the line of +# sight, x to the left, y up. +# * oops-frame: the oops observation frame that the FOV uses. Axes follow the +# oops convention: z along the line of sight, x to the right, y down. +# oops-frame = CMATRIX_ROTATION * spice-frame. The instrument's internal coordinate system +# matches the oops-frame, so a recorded (spice-frame) C-matrix is rotated by +# CMATRIX_ROTATION at the boundary to build the observation frame. +CMATRIX_ROTATION = oops.Matrix3([[-1,0,0],[0,-1,0],[0,0,1]]) ################################################################################ # Standard class methods @@ -41,13 +45,13 @@ def from_file(filespec, fast_distortion=True, return_all_planets Include kernels for all planets not just Jupiter or Saturn. - cmatrix an oops.Matrix3 (or 3x3 array) giving the SPICE - camera-frame C-matrix: the rotation from J2000 into - the SPICE CASSINI_ISS_ frame, exactly as - returned by cspyce.pxform() or recorded in a CK. The - fixed 180-degree instrument rotation relating that - frame to the oops observation frame is applied - internally. None uses SPICE pointing as before. + cmatrix an oops.Matrix3 (or 3x3 array) giving the spice-frame + C-matrix: the rotation from J2000 into the SPICE + CASSINI_ISS_ frame, exactly as returned by + cspyce.pxform() or recorded in a CK (z along the line + of sight, x left, y up). The fixed 180-degree rotation + relating it to the oops-frame observation frame is + applied internally. None uses SPICE pointing as before. frame_id when cmatrix is given and map_other_camera is False, register the C-matrix under this frame ID and use only @@ -121,6 +125,7 @@ def from_file(filespec, fast_distortion=True, frame = frame, dict = vicar_dict, # Add the VICAR dict data = vic.data_2d, # Add the data array + host = ISS, # Add the host class instrument = 'ISS', detector = camera, sampling = mode, @@ -173,6 +178,7 @@ def from_index(filespec, **parameters): 'CASSINI', 'CASSINI_ISS_' + camera, dict = row_dict, # Add index dictionary index_dict = row_dict, # Old name + host = ISS, # Add the host class instrument = 'ISS', detector = camera, sampling = mode) @@ -233,6 +239,11 @@ class ISS(object): frames_defined = False offset_wac = False + # Exposed on the class so the generic Observation.get_cmatrix() can read the + # host's oops-frame -> spice-frame convention rotation via the observation's + # `host` subfield. See the module-level CMATRIX_ROTATION definition above. + CMATRIX_ROTATION = CMATRIX_ROTATION + # Create a master version of the NAC and WAC distortion models from # Owen Jr., W.M., 2003. Cassini ISS Geometric Calibration of April 2003. # JPL IOM 312.E-2003. @@ -443,11 +454,11 @@ def define_camera_frames(): # Construct a SpiceFrame for each camera # Deal with the fact that the instrument's internal - # coordinate system is rotated 180 degrees (see ROT180) - nac_flipped = oops.frame.SpiceFrame('CASSINI_ISS_NAC', - frame_id='CASSINI_ISS_NAC_FLIPPED') - wac_flipped = oops.frame.SpiceFrame('CASSINI_ISS_WAC', - frame_id='CASSINI_ISS_WAC_FLIPPED') + # coordinate system is rotated 180 degrees (see CMATRIX_ROTATION) + nac_frame_spice = oops.frame.SpiceFrame('CASSINI_ISS_NAC', + frame_id='CASSINI_ISS_NAC_SPICE') + wac_frame_spice = oops.frame.SpiceFrame('CASSINI_ISS_WAC', + frame_id='CASSINI_ISS_WAC_SPICE') if ISS.offset_wac: @@ -470,94 +481,97 @@ def define_camera_frames(): # frames are never replaced by a custom C-matrix and remain a reliable # reference for the inter-camera rotation (see set_cmatrix). override=True # so a rebuild after ISS.reset() refreshes them to the current pointing. - oops.frame.Cmatrix(ROT180, nac_flipped, + oops.frame.Cmatrix(CMATRIX_ROTATION, nac_frame_spice, frame_id='CASSINI_ISS_NAC_SPICE', override=True) if ISS.offset_wac: - wac_spice_no = oops.frame.Cmatrix(ROT180, wac_flipped, + wac_frame_spice_raw = oops.frame.Cmatrix(CMATRIX_ROTATION, wac_frame_spice, frame_id='CASSINI_ISS_WAC-NO_OFFSET_SPICE', override=True) - oops.frame.Navigation((xshift,yshift), wac_spice_no, + oops.frame.Navigation((xshift,yshift), wac_frame_spice_raw, frame_id='CASSINI_ISS_WAC_SPICE', override=True) else: - oops.frame.Cmatrix(ROT180, wac_flipped, + oops.frame.Cmatrix(CMATRIX_ROTATION, wac_frame_spice, frame_id='CASSINI_ISS_WAC_SPICE', override=True) # Observation-facing camera frames. These start out identical to the # *_SPICE frames above, but set_cmatrix may later replace them with a # custom C-matrix. override=True: a prior custom C-matrix may already # have replaced the primary definition of this frame ID; reclaim it. - nac_frame = oops.frame.Cmatrix(ROT180, nac_flipped, + nac_frame = oops.frame.Cmatrix(CMATRIX_ROTATION, nac_frame_spice, frame_id='CASSINI_ISS_NAC', override=True) if ISS.offset_wac: - wac_frame_no = oops.frame.Cmatrix(ROT180, wac_flipped, + wac_frame_spice_raw = oops.frame.Cmatrix(CMATRIX_ROTATION, wac_frame_spice, frame_id='CASSINI_ISS_WAC-NO_OFFSET', override=True) - wac_frame = oops.frame.Navigation((xshift,yshift), wac_frame_no, + wac_frame = oops.frame.Navigation((xshift,yshift), wac_frame_spice_raw, frame_id='CASSINI_ISS_WAC', override=True) else: - wac_frame = oops.frame.Cmatrix(ROT180, wac_flipped, + wac_frame = oops.frame.Cmatrix(CMATRIX_ROTATION, wac_frame_spice, frame_id='CASSINI_ISS_WAC', override=True) ISS.frames_defined = True #=========================================================================== @staticmethod - def oops_from_host(cmatrix): - """Convert a host C-matrix into the oops observation-frame attitude. + def oops_from_spice(cmatrix): + """Convert a spice-frame C-matrix into the oops-frame attitude. - The host convention is the SPICE camera-frame C-matrix: the rotation - from J2000 into SPICE's CASSINI_ISS_ frame, as returned by - cspyce.pxform() or recorded in a CK. The oops convention is the attitude - of the oops observation frame (what Observation.cmatrix() returns), which - is that SPICE camera frame rotated 180 degrees about the boresight. + The spice-frame is the SPICE camera-frame C-matrix: the rotation from + J2000 into SPICE's CASSINI_ISS_ frame, as returned by + cspyce.pxform() or recorded in a CK (z along the line of sight, x left, + y up). The oops-frame is the oops observation frame, that SPICE frame + rotated 180 degrees about the boresight (z along the line of sight, x + right, y down). Input: - cmatrix an oops.Matrix3 (or 3x3 array) in the host (SPICE - camera-frame) convention. + cmatrix an oops.Matrix3 (or 3x3 array) in the spice-frame + convention. - Return: an oops.Matrix3 in the oops observation-frame convention. + Return: an oops.Matrix3 in the oops-frame convention. """ - return ROT180 * oops.Matrix3.as_matrix3(cmatrix) + return CMATRIX_ROTATION * oops.Matrix3.as_matrix3(cmatrix) #=========================================================================== @staticmethod - def host_from_oops(cmatrix): - """Convert an oops observation-frame attitude into a host C-matrix. + def spice_from_oops(cmatrix): + """Convert an oops-frame attitude into a spice-frame C-matrix. - The inverse of oops_from_host(); see that method for the two conventions. - Because the frames differ by a 180-degree rotation (its own inverse), - this applies the same rotation. + The inverse of oops_from_spice(); see that method for the two + conventions and their axes. Because the frames differ by a 180-degree + rotation (its own inverse), this applies the same rotation. Input: - cmatrix an oops.Matrix3 (or 3x3 array) in the oops - observation-frame convention (e.g. Observation.cmatrix()). + cmatrix an oops.Matrix3 (or 3x3 array) in the oops-frame + convention (z along the line of sight, x right, y down). - Return: an oops.Matrix3 in the host (SPICE camera-frame) - convention. + Return: an oops.Matrix3 in the spice-frame convention (z along + the line of sight, x left, y up). """ - return ROT180 * oops.Matrix3.as_matrix3(cmatrix) + return CMATRIX_ROTATION * oops.Matrix3.as_matrix3(cmatrix) #=========================================================================== @staticmethod def set_cmatrix(cmatrix, camera, frame_id=None, map_other_camera=False, time=None): - """Build an oops observation frame from a custom SPICE camera-frame - C-matrix and return the frame to attach to the observation. + """Build an oops observation frame from a custom spice-frame C-matrix + and return the frame to attach to the observation. - ISS.initialize() must have been called first. The C-matrix is the SPICE - camera-frame pointing (the rotation from J2000 into SPICE's + ISS.initialize() must have been called first. The C-matrix is the + spice-frame pointing (the rotation from J2000 into SPICE's CASSINI_ISS_ frame, as returned by cspyce.pxform() or recorded in - a CK). The oops observation frame is that frame rotated 180 degrees about - the boresight (see ROT180); this rotation is applied here, at the - boundary, so a recorded C-matrix reproduces the observation's pointing. + a CK; z along the line of sight, x left, y up). The oops-frame + observation frame is that frame rotated 180 degrees about the boresight + (see CMATRIX_ROTATION; z along the line of sight, x right, y down); this rotation + is applied here, at the boundary, so a recorded C-matrix reproduces the + observation's pointing. Input: cmatrix an oops.Matrix3 (or 3x3 array) for the named camera, in - the SPICE camera-frame convention. + the spice-frame convention. camera 'NAC' or 'WAC', the camera the cmatrix applies to. frame_id when map_other_camera is False: None (default) returns a fresh unregistered frame owned by the observation, so @@ -575,9 +589,9 @@ def set_cmatrix(cmatrix, camera, frame_id=None, map_other_camera=False, ID (frame_id given, or map_other_camera). """ - # Convert the SPICE camera-frame C-matrix into the oops observation-frame + # Convert the spice-frame C-matrix into the oops-frame observation-frame # attitude by applying the fixed 180-degree instrument rotation. - attitude = ISS.oops_from_host(cmatrix) + attitude = ISS.oops_from_spice(cmatrix) if map_other_camera: # Build the SPICE-derived frames first (so the dedicated *_SPICE @@ -650,30 +664,6 @@ def map_other_camera(camera, time): return 'CASSINI_ISS_' + camera - #=========================================================================== - @staticmethod - def get_cmatrix(obs, uv=None, time=None): - """The SPICE camera-frame C-matrix of an ISS observation. - - The inverse of set_cmatrix(): it returns the observation's pointing in - the SPICE camera-frame convention (the rotation from J2000 into SPICE's - CASSINI_ISS_ frame, as cspyce.pxform() would return), so feeding - the result back into from_file(cmatrix=...) reproduces the pointing. - - Input: - obs an ISS observation (e.g. a Snapshot from from_file); its - frame must be the oops ISS observation frame. - uv a (u,v) pixel location, used only to select a time when - the observation frame is time-dependent; None to use the - center of the FOV. Ignored if time is given. - time the time in seconds TDB at which to evaluate the frame; - None to derive it from uv. - - Return: an oops.Matrix3, the SPICE camera-frame C-matrix. - """ - - return ISS.host_from_oops(obs.cmatrix(uv=uv, time=time)) - #=========================================================================== @staticmethod def reset(): diff --git a/oops/observation/observation_.py b/oops/observation/observation_.py index eea87963..baa414d6 100755 --- a/oops/observation/observation_.py +++ b/oops/observation/observation_.py @@ -452,40 +452,38 @@ def midtime_at_uv(self, uv, tfrac=0.5): return tfrac * (time0 + time1) #=========================================================================== - def cmatrix(self, uv=None, time=None, reference=None): - """The 3x3 rotation matrix that rotates a reference frame into this - observation's frame at a selected time. - - This returns the attitude of the observation frame itself (by default, - the J2000 -> observation-frame rotation). Note that a host's "cmatrix" - input may use a different convention: e.g. - oops.hosts.cassini.iss.from_file(cmatrix=...) takes the SPICE - camera-frame C-matrix, which for ISS differs from the observation frame - by a fixed 180-degree rotation about the boresight. The value returned - here is therefore not, in general, interchangeable with such an input. + def get_cmatrix(self, uv=None, time=None): + """The host-convention C-matrix of this observation. + + This derives the observation's pointing from its own frame and the fixed + convention rotation exposed by its host (the 'host' subfield's CMATRIX_ROTATION + attribute): it evaluates the oops observation-frame attitude and applies + the host rotation. For ISS the result is the spice-frame C-matrix: the + rotation from J2000 into SPICE's CASSINI_ISS_ frame, as + cspyce.pxform() returns, with z along the line of sight, x left, y up. + Note this is NOT the oops-frame convention (z along the line of sight, x + right, y down) of the observation frame itself; the two differ by that + 180-degree rotation about the boresight. Because it is derived from + self.frame at call time, the result reflects this observation's own + pointing and is unaffected by other loads. Input: - uv a (u,v) pixel location, used only to select a time - when this observation's frame is time-dependent (e.g., - a slewing spacecraft during a raster or TDI exposure); - None to use the center of the FOV. Ignored if time is - given explicitly. - time the time in seconds TDB at which to evaluate the - frame; None to derive it from uv via midtime_at_uv(). - reference the frame or frame ID that the returned matrix rotates - from; None for J2000. - - Return: a Matrix3 giving the rotation from the reference frame - into this observation's frame at the selected time. + uv a (u,v) pixel location, used only to select a time when + this observation's frame is time-dependent; None to use + the center of the FOV. Ignored if time is given. + time the time in seconds TDB at which to evaluate the frame; + None to derive it from uv via midtime_at_uv(). + + Return: a Matrix3 giving the host-convention C-matrix (for ISS, + the spice-frame C-matrix). """ if time is None: uv = self.fov.uv_shape / 2. if uv is None else Pair.as_pair(uv) time = self.midtime_at_uv(uv) - reference = Frame.as_wayframe(reference) or Frame.J2000 - xform = self.frame.wrt(reference).transform_at_time(time) - return xform.matrix + oops_attitude = self.frame.wrt(Frame.J2000).transform_at_time(time).matrix + return self.host.CMATRIX_ROTATION * oops_attitude #=========================================================================== def meshgrid(self, origin=None, undersample=1, oversample=1, limit=None, diff --git a/tests/hosts/cassini/iss/test_iss.py b/tests/hosts/cassini/iss/test_iss.py index cbd1b6ad..d7880983 100644 --- a/tests/hosts/cassini/iss/test_iss.py +++ b/tests/hosts/cassini/iss/test_iss.py @@ -9,7 +9,7 @@ from oops.body import Body from oops.frame import Frame from oops.hosts.cassini import Cassini -from oops.hosts.cassini.iss import ISS, from_file, ROT180 +from oops.hosts.cassini.iss import ISS, from_file, CMATRIX_ROTATION from oops.unittester_support import TEST_DATA_PREFIX # A rotation distinguishable from any SPICE-derived pointing, used to build @@ -20,13 +20,12 @@ class Test_Cassini_ISS_Cmatrix(unittest.TestCase): """Tests for the custom C-matrix support in hosts/cassini/iss.py (from_file's cmatrix/frame_id/map_other_camera arguments and - ISS.set_cmatrix), plus the generic Observation.cmatrix() getter. + ISS.set_cmatrix), plus the generic Observation.get_cmatrix() getter. The `cmatrix` accepted by from_file is the SPICE camera-frame C-matrix (the J2000 -> CASSINI_ISS_ rotation, as returned by cspyce.pxform). The - oops observation frame is that frame rotated 180 degrees about the boresight, - so an observation's attitude (Observation.cmatrix()) equals ROT180 times the - SPICE C-matrix that produced it. + oops observation frame is that frame rotated 180 degrees about the boresight; + get_cmatrix() returns the pointing back in that SPICE C-matrix convention. """ FILESPEC = 'cassini/ISS/W1573721822_1.IMG' # a WAC image @@ -57,55 +56,21 @@ def _spice_cmatrix(self, camera, time): return (Frame.as_wayframe('CASSINI_ISS_' + camera + '_FLIPPED') .wrt(Frame.J2000).transform_at_time(time).matrix) - #=========================================================================== - def test_pxform_roundtrip(self): - """Feeding the recorded SPICE C-matrix (what pxform returns) straight - into from_file reproduces the SPICE pointing exactly, because the fixed - 180-degree instrument rotation is applied at the boundary.""" - - baseline = from_file(self.filespec) # SPICE pointing - camera = baseline.dict['INSTRUMENT_ID'][3:] + 'C' - spice_cmatrix = self._spice_cmatrix(camera, baseline.tstart) - - obs = from_file(self.filespec, cmatrix=spice_cmatrix) - - # Same pointing as the SPICE-derived observation: zero offset. - self.assertTrue(np.allclose(obs.cmatrix().vals, baseline.cmatrix().vals)) - - # The boundary rotation really was applied: the observation-frame - # attitude is ROT180 times the raw SPICE C-matrix, not the C-matrix - # itself (the two differ by the full 180 degrees). - self.assertTrue(np.allclose(obs.cmatrix().vals, - (ROT180 * spice_cmatrix).vals)) - self.assertFalse(np.allclose(obs.cmatrix().vals, spice_cmatrix.vals)) - #=========================================================================== def test_convention_conversions(self): - """ISS.oops_from_host / host_from_oops convert between the two - conventions, are mutual inverses, and match the from_file / - Observation.cmatrix() endpoints.""" + """ISS.oops_from_spice / spice_from_oops convert between the two + conventions and are mutual inverses.""" # Mutual inverses, for an arbitrary rotation. - m = _PERTURBATION * ROT180 * _PERTURBATION + m = _PERTURBATION * CMATRIX_ROTATION * _PERTURBATION self.assertTrue(np.allclose(m.vals, - ISS.host_from_oops(ISS.oops_from_host(m)).vals)) + ISS.spice_from_oops(ISS.oops_from_spice(m)).vals)) self.assertTrue(np.allclose(m.vals, - ISS.oops_from_host(ISS.host_from_oops(m)).vals)) - - # They tie the SPICE C-matrix (pxform) to the oops observation-frame - # attitude used by from_file() and returned by Observation.cmatrix(). - baseline = from_file(self.filespec) - camera = baseline.dict['INSTRUMENT_ID'][3:] + 'C' - spice_cmatrix = self._spice_cmatrix(camera, baseline.tstart) - - self.assertTrue(np.allclose(ISS.oops_from_host(spice_cmatrix).vals, - baseline.cmatrix().vals)) - self.assertTrue(np.allclose(ISS.host_from_oops(baseline.cmatrix()).vals, - spice_cmatrix.vals)) + ISS.oops_from_spice(ISS.spice_from_oops(m)).vals)) #=========================================================================== def test_get_cmatrix_inverts_set(self): - """ISS.get_cmatrix returns the SPICE-convention C-matrix (matching + """get_cmatrix returns the SPICE-convention C-matrix (matching pxform) and inverts the cmatrix given to from_file.""" baseline = from_file(self.filespec) @@ -114,18 +79,19 @@ def test_get_cmatrix_inverts_set(self): # For a SPICE-loaded observation, get_cmatrix matches pxform. self.assertTrue(np.allclose( - ISS.get_cmatrix(baseline, time=baseline.tstart).vals, spice0.vals)) + baseline.get_cmatrix(time=baseline.tstart).vals, spice0.vals)) # It recovers the exact cmatrix given to from_file (the custom Snapshot # frame is fixed in time, so no time argument is needed). custom = _PERTURBATION * spice0 obs = from_file(self.filespec, cmatrix=custom) - recovered = ISS.get_cmatrix(obs) + recovered = obs.get_cmatrix() self.assertTrue(np.allclose(recovered.vals, custom.vals)) # Round-trip: feeding it back into from_file reproduces the pointing. obs2 = from_file(self.filespec, cmatrix=recovered) - self.assertTrue(np.allclose(obs2.cmatrix().vals, obs.cmatrix().vals)) + self.assertTrue(np.allclose(obs2.get_cmatrix().vals, + obs.get_cmatrix().vals)) #=========================================================================== def test_default_frame_is_unregistered_and_isolated(self): @@ -138,20 +104,21 @@ def test_default_frame_is_unregistered_and_isolated(self): custom1 = _PERTURBATION * spice0 obs1 = from_file(self.filespec, cmatrix=custom1) - saved1 = obs1.cmatrix() + saved1 = obs1.get_cmatrix() # The observation owns an unregistered frame, not the global wayframe. self.assertFalse(obs1.frame.is_registered()) - self.assertTrue(np.allclose(obs1.cmatrix().vals, (ROT180 * custom1).vals)) + self.assertTrue(np.allclose(obs1.get_cmatrix().vals, custom1.vals)) # Loading a second custom image must not disturb the first. custom2 = _PERTURBATION * _PERTURBATION * spice0 obs2 = from_file(self.filespec, cmatrix=custom2) self.assertIsNot(obs1.frame, obs2.frame) - self.assertTrue(np.all(obs1.cmatrix().vals == saved1.vals)) - self.assertTrue(np.allclose(obs2.cmatrix().vals, (ROT180 * custom2).vals)) - self.assertFalse(np.allclose(obs1.cmatrix().vals, obs2.cmatrix().vals)) + self.assertTrue(np.all(obs1.get_cmatrix().vals == saved1.vals)) + self.assertTrue(np.allclose(obs2.get_cmatrix().vals, custom2.vals)) + self.assertFalse(np.allclose(obs1.get_cmatrix().vals, + obs2.get_cmatrix().vals)) #=========================================================================== def test_default_does_not_leak_into_plain_load(self): @@ -159,24 +126,22 @@ def test_default_does_not_leak_into_plain_load(self): subsequent plain load reads back its own SPICE pointing.""" baseline = from_file(self.filespec) - m0 = baseline.cmatrix() + m0 = baseline.get_cmatrix() camera = baseline.dict['INSTRUMENT_ID'][3:] + 'C' spice0 = self._spice_cmatrix(camera, baseline.tstart) _ = from_file(self.filespec, cmatrix=_PERTURBATION * spice0) plain = from_file(self.filespec) - self.assertTrue(np.allclose(plain.cmatrix().vals, m0.vals)) + self.assertTrue(np.allclose(plain.get_cmatrix().vals, m0.vals)) #=========================================================================== def test_explicit_frame_id_leaves_global_untouched(self): """A custom cmatrix with an explicit frame_id registers a separate - frame (with the boundary rotation applied) and leaves the global - CASSINI_ISS_ frame alone.""" + frame (with the boundary rotation applied) under that frame ID.""" baseline = from_file(self.filespec) camera = baseline.dict['INSTRUMENT_ID'][3:] + 'C' - m0 = baseline.cmatrix() spice0 = self._spice_cmatrix(camera, baseline.tstart) custom = _PERTURBATION * spice0 @@ -184,11 +149,12 @@ def test_explicit_frame_id_leaves_global_untouched(self): frame_id='TEST_ISS_CUSTOM_FRAME') self.assertEqual(obs.frame.frame_id, 'TEST_ISS_CUSTOM_FRAME') - self.assertTrue(np.allclose(obs.cmatrix().vals, (ROT180 * custom).vals)) + self.assertTrue(np.allclose(obs.get_cmatrix().vals, custom.vals)) # The global camera frame is untouched. unaffected = from_file(self.filespec) - self.assertTrue(np.allclose(unaffected.cmatrix().vals, m0.vals)) + self.assertTrue(np.allclose(unaffected.get_cmatrix().vals, + baseline.get_cmatrix().vals)) #=========================================================================== def test_map_other_camera(self): @@ -209,7 +175,6 @@ def test_map_other_camera(self): obs = from_file(self.filespec, cmatrix=custom, map_other_camera=True) self.assertEqual(obs.frame.frame_id, 'CASSINI_ISS_' + camera) - self.assertTrue(np.allclose(obs.cmatrix().vals, (ROT180 * custom).vals)) # The fixed inter-camera rotation is preserved under the override. new_rel = (Frame.as_wayframe('CASSINI_ISS_' + other) @@ -222,7 +187,7 @@ def test_map_other_camera(self): .wrt(Frame.J2000) .transform_at_time(baseline.tstart).matrix) self.assertTrue(np.allclose(other_matrix.vals, - (baseline_rel * ROT180 * custom).vals)) + (baseline_rel * CMATRIX_ROTATION * custom).vals)) #=========================================================================== def test_map_reclaims_global_on_plain_load(self): @@ -231,7 +196,7 @@ def test_map_reclaims_global_on_plain_load(self): custom pointing.""" baseline = from_file(self.filespec) - m0 = baseline.cmatrix() + m0 = baseline.get_cmatrix() camera = baseline.dict['INSTRUMENT_ID'][3:] + 'C' spice0 = self._spice_cmatrix(camera, baseline.tstart) @@ -239,7 +204,7 @@ def test_map_reclaims_global_on_plain_load(self): map_other_camera=True) plain = from_file(self.filespec) - self.assertTrue(np.allclose(plain.cmatrix().vals, m0.vals)) + self.assertTrue(np.allclose(plain.get_cmatrix().vals, m0.vals)) #=========================================================================== def test_map_after_map_override(self): @@ -265,15 +230,13 @@ def test_map_after_map_override(self): # Second mapped load: rel must come from the dedicated *_SPICE frames, # unaffected by the override above. custom = _PERTURBATION * spice0 - obs = from_file(self.filespec, cmatrix=custom, map_other_camera=True) - - self.assertTrue(np.allclose(obs.cmatrix().vals, (ROT180 * custom).vals)) + _ = from_file(self.filespec, cmatrix=custom, map_other_camera=True) other_matrix = (Frame.as_wayframe('CASSINI_ISS_' + other) .wrt(Frame.J2000) .transform_at_time(baseline.tstart).matrix) self.assertTrue(np.allclose(other_matrix.vals, - (baseline_rel * ROT180 * custom).vals)) + (baseline_rel * CMATRIX_ROTATION * custom).vals)) #=========================================================================== def test_no_ck_loaded_without_mapping(self): @@ -307,7 +270,7 @@ def test_reset_restores_spice_pointing(self): SPICE-derived pointing even if a custom cmatrix was used earlier.""" baseline = from_file(self.filespec) - m0 = baseline.cmatrix() + m0 = baseline.get_cmatrix() camera = baseline.dict['INSTRUMENT_ID'][3:] + 'C' spice0 = self._spice_cmatrix(camera, baseline.tstart) @@ -316,7 +279,7 @@ def test_reset_restores_spice_pointing(self): ISS.reset() restored = from_file(self.filespec) - self.assertTrue(np.allclose(restored.cmatrix().vals, m0.vals)) + self.assertTrue(np.allclose(restored.get_cmatrix().vals, m0.vals)) ############################################ if __name__ == '__main__': diff --git a/tests/observation/test_snapshot.py b/tests/observation/test_snapshot.py index e10e3275..cc71e8bf 100755 --- a/tests/observation/test_snapshot.py +++ b/tests/observation/test_snapshot.py @@ -5,9 +5,8 @@ import numpy as np import unittest -from polymath import Matrix3, Pair, Scalar, Vector +from polymath import Pair, Vector from oops.fov import FlatFOV -from oops.frame import Cmatrix, SpinFrame from oops.observation import Snapshot @@ -125,58 +124,6 @@ def runTest(self): self.assertEqual(uv[:4], indices.to_pair((2,0))[:4]) self.assertTrue(np.all(uv.mask == 4*[False] + [True])) - # cmatrix() - m = Matrix3([[0,1,0],[0,0,-1],[-1,0,0]]) - cmatrix_frame = Cmatrix(m, frame_id='TEST_SNAPSHOT_CMATRIX') - - obs = Snapshot(('u','v'), tstart=98., texp=2., - fov=fov, path='SSB', frame=cmatrix_frame) - - self.assertTrue(np.all(obs.cmatrix().vals == m.vals)) - self.assertTrue(np.all(obs.cmatrix(uv=(3,4)).vals == m.vals)) - self.assertTrue(np.all(obs.cmatrix(time=99.).vals == m.vals)) - self.assertTrue(np.all(obs.cmatrix(reference=obs.frame).vals - == np.eye(3))) - - # cmatrix() with a time-dependent frame, using both uv and time - # - # The Cmatrix frame above is fixed in time, so it cannot reveal whether - # cmatrix() actually consults uv and time. Here we combine a rotating - # SpinFrame (a different matrix at every time) with an observation whose - # midtime_at_uv() varies with the u coordinate, so distinct UVs must map - # to distinct times and therefore distinct matrices. - - spin = SpinFrame(0., 1., 0., 2, 'J2000', - frame_id='TEST_SNAPSHOT_SPIN') - - def spin_matrix(t): - # The J2000 -> SpinFrame rotation at time t (see SpinFrame, axis=2) - (c, s) = (np.cos(t), np.sin(t)) - return np.array([[c, s, 0.], [-s, c, 0.], [0., 0., 1.]]) - - class TimeDependentSnapshot(Snapshot): - # midtime depends on the u coordinate: midtime == u - def midtime_at_uv(self, uv, tfrac=0.5): - return Scalar.as_scalar(Pair.as_pair(uv).to_scalar(0)) - - obs = TimeDependentSnapshot(('u','v'), tstart=98., texp=2., - fov=fov, path='SSB', frame=spin) - - # Selecting different UVs selects different times -> different matrices - cmat1 = obs.cmatrix(uv=(1.,4.)) - cmat2 = obs.cmatrix(uv=(2.,4.)) - self.assertTrue(np.allclose(cmat1.vals, spin_matrix(1.))) - self.assertTrue(np.allclose(cmat2.vals, spin_matrix(2.))) - self.assertFalse(np.allclose(cmat1.vals, cmat2.vals)) - - # The default uv is the center of the FOV (uv_shape/2 -> u == 5) - self.assertTrue(np.allclose(obs.cmatrix().vals, spin_matrix(5.))) - - # An explicit time is used directly, overriding uv - self.assertTrue(np.allclose(obs.cmatrix(time=3.).vals, spin_matrix(3.))) - self.assertTrue(np.allclose(obs.cmatrix(uv=(1.,4.), time=3.).vals, - spin_matrix(3.))) - ################################################################################ if __name__ == '__main__': unittest.main(verbosity=2) From 86c7635fe10cd3de264e3b69385a3b99fc87198f Mon Sep 17 00:00:00 2001 From: Joseph Spitale Date: Wed, 12 Aug 2026 15:33:38 -0700 Subject: [PATCH 11/22] Add generic Observation.set_cmatrix from a spice-convention C-matrix Mirrors iss.set_cmatrix as the inverse of get_cmatrix, applying the host's CMATRIX_ROTATION to build an observation frame; supports an optional frame_id, without the map_other_camera machinery. Co-Authored-By: Claude Opus 4.8 --- oops/observation/observation_.py | 49 ++++++++++++++++++++++++++++++-- 1 file changed, 46 insertions(+), 3 deletions(-) diff --git a/oops/observation/observation_.py b/oops/observation/observation_.py index baa414d6..52e03895 100755 --- a/oops/observation/observation_.py +++ b/oops/observation/observation_.py @@ -5,10 +5,11 @@ import numpy as np import numbers -from polymath import Scalar, Pair, Vector, Vector3, Qube +from polymath import Scalar, Pair, Vector, Vector3, Qube, Matrix3 from oops.config import LOGGING, PATH_PHOTONS from oops.event import Event from oops.frame import Frame +from oops.frame.cmatrix import Cmatrix from oops.frame.navigation import Navigation from oops.meshgrid import Meshgrid @@ -453,7 +454,7 @@ def midtime_at_uv(self, uv, tfrac=0.5): #=========================================================================== def get_cmatrix(self, uv=None, time=None): - """The host-convention C-matrix of this observation. + """The spice-convention C-matrix of this observation. This derives the observation's pointing from its own frame and the fixed convention rotation exposed by its host (the 'host' subfield's CMATRIX_ROTATION @@ -474,7 +475,7 @@ def get_cmatrix(self, uv=None, time=None): time the time in seconds TDB at which to evaluate the frame; None to derive it from uv via midtime_at_uv(). - Return: a Matrix3 giving the host-convention C-matrix (for ISS, + Return: a Matrix3 giving the spice-convention C-matrix (for ISS, the spice-frame C-matrix). """ @@ -485,6 +486,48 @@ def get_cmatrix(self, uv=None, time=None): oops_attitude = self.frame.wrt(Frame.J2000).transform_at_time(time).matrix return self.host.CMATRIX_ROTATION * oops_attitude + #=========================================================================== + def set_cmatrix(self, cmatrix, frame_id=None): + """Build an oops observation frame from a spice-convention C-matrix and + return the frame to attach to the observation. + + This is the inverse of get_cmatrix(): it takes the spice-convention + C-matrix (for ISS, the spice-frame pointing: the rotation from J2000 + into SPICE's CASSINI_ISS_ frame, with z along the line of sight, + x left, y up) and converts it into the oops observation-frame attitude by + applying the fixed convention rotation exposed by the observation's host + (the 'host' subfield's CMATRIX_ROTATION attribute; z along the line of + sight, x right, y down). Because that rotation is its own inverse, the + same product recovers the observation attitude from the C-matrix that + get_cmatrix() applied to produce it. + + Input: + cmatrix an oops.Matrix3 (or 3x3 array) in the spice-convention + C-matrix, as returned by get_cmatrix(). + frame_id None (default) returns a fresh unregistered frame owned by + the observation, so loading other images never disturbs + its pointing; otherwise the frame is registered under this + frame ID (a shared, registered frame). + + Return: the frame to attach to the observation: a Cmatrix frame + object (unregistered default case) or a registered frame + ID (frame_id given). + """ + + # Convert the spice-convention C-matrix into the oops observation-frame + # attitude by applying the fixed instrument convention rotation. + attitude = self.host.CMATRIX_ROTATION * Matrix3.as_matrix3(cmatrix) + + if frame_id is not None: + # Register a single, shared custom frame; global frames untouched. + Cmatrix(attitude, frame_id=frame_id) + return frame_id + + # Default: a fresh unregistered frame owned by this observation. It never + # enters the global registry, so it neither collides with nor is + # overwritten by any other image's pointing. + return Cmatrix(attitude, frame_id=None) + #=========================================================================== def meshgrid(self, origin=None, undersample=1, oversample=1, limit=None, center_uv=None, fov_keywords={}): From e1faddbd050d627ca4a415ce446a9fd892863c02 Mon Sep 17 00:00:00 2001 From: Joseph Spitale Date: Wed, 12 Aug 2026 16:23:41 -0700 Subject: [PATCH 12/22] Remove map_other_camera and its dedicated *_SPICE reference frames Only the label camera's frame is ever created; a custom C-matrix never touches the other camera or the global frame registry. Co-Authored-By: Claude Fable 5 --- oops/hosts/cassini/iss.py | 141 +++++----------------------- tests/hosts/cassini/iss/test_iss.py | 125 ++---------------------- 2 files changed, 34 insertions(+), 232 deletions(-) diff --git a/oops/hosts/cassini/iss.py b/oops/hosts/cassini/iss.py index e8254752..6664cdc5 100755 --- a/oops/hosts/cassini/iss.py +++ b/oops/hosts/cassini/iss.py @@ -31,7 +31,7 @@ def from_file(filespec, fast_distortion=True, return_all_planets=False, cmatrix=None, frame_id=None, - map_other_camera=False, **parameters): + **parameters): """A general, static method to return a Snapshot object based on a given Cassini ISS image file. @@ -53,17 +53,11 @@ def from_file(filespec, fast_distortion=True, relating it to the oops-frame observation frame is applied internally. None uses SPICE pointing as before. - frame_id when cmatrix is given and map_other_camera is False, - register the C-matrix under this frame ID and use only - that (shared, registered) frame. None (default) gives - the observation its own unregistered frame, so loading - other images never disturbs this one's pointing. - - map_other_camera when True, also derive and register the other camera's - frame from the fixed inter-camera rotation (uses - SPICE); frame_id is ignored. When False (default), - only the label camera's frame is created and SPICE - pointing is completely circumvented (no CK loaded). + frame_id when cmatrix is given, register the C-matrix under + this frame ID and use only that (shared, registered) + frame. None (default) gives the observation its own + unregistered frame, so loading other images never + disturbs this one's pointing. """ ISS.initialize() # Define everything the first time through; use defaults @@ -102,18 +96,16 @@ def from_file(filespec, fast_distortion=True, # Make sure the SPICE kernels are loaded. SPKs are always needed for the # spacecraft path; CKs (pointing) are only needed when the pointing comes - # from SPICE, i.e. no custom cmatrix, or when mapping to the other camera. + # from SPICE, i.e. no custom cmatrix. Cassini.load_spks(tstart, tstart + texp) - if cmatrix is None or map_other_camera: + if cmatrix is None: Cassini.load_cks(tstart, tstart + texp) # Determine the observation frame, applying a custom C-matrix if given. - # ISS.set_cmatrix builds the frame(s) and returns the one to attach to this - # observation; SPICE pointing is circumvented unless the other camera is - # being mapped. + # ISS.set_cmatrix builds the frame and returns the one to attach to this + # observation; SPICE pointing is circumvented. if cmatrix is not None: - frame = ISS.set_cmatrix(cmatrix, camera, frame_id=frame_id, - map_other_camera=map_other_camera, time=tstart) + frame = ISS.set_cmatrix(cmatrix, camera, frame_id=frame_id) else: ISS.define_camera_frames() # use the SPICE-derived pointing frame = 'CASSINI_ISS_' + camera @@ -444,8 +436,8 @@ def define_camera_frames(): """Register the SPICE-derived CASSINI_ISS_NAC and CASSINI_ISS_WAC frames. ISS.initialize() must have been called first. Built lazily (and only - once) so that observations using a custom C-matrix without mapping never - construct or depend on the SPICE camera frames. + once) so that observations using a custom C-matrix never construct or + depend on the SPICE camera frames. """ # Quick exit after first call @@ -476,27 +468,12 @@ def define_camera_frames(): xshift = -7. * xpixel yshift = 4.4 * ypixel - # Dedicated SPICE-derived image frames, kept under their own frame IDs. - # set_cmatrix only ever overrides CASSINI_ISS_NAC/WAC, so these *_SPICE - # frames are never replaced by a custom C-matrix and remain a reliable - # reference for the inter-camera rotation (see set_cmatrix). override=True - # so a rebuild after ISS.reset() refreshes them to the current pointing. - oops.frame.Cmatrix(CMATRIX_ROTATION, nac_frame_spice, - frame_id='CASSINI_ISS_NAC_SPICE', override=True) - if ISS.offset_wac: - wac_frame_spice_raw = oops.frame.Cmatrix(CMATRIX_ROTATION, wac_frame_spice, - frame_id='CASSINI_ISS_WAC-NO_OFFSET_SPICE', - override=True) - oops.frame.Navigation((xshift,yshift), wac_frame_spice_raw, - frame_id='CASSINI_ISS_WAC_SPICE', override=True) - else: - oops.frame.Cmatrix(CMATRIX_ROTATION, wac_frame_spice, - frame_id='CASSINI_ISS_WAC_SPICE', override=True) - - # Observation-facing camera frames. These start out identical to the - # *_SPICE frames above, but set_cmatrix may later replace them with a - # custom C-matrix. override=True: a prior custom C-matrix may already - # have replaced the primary definition of this frame ID; reclaim it. + # Observation-facing camera frames: the raw *_SPICE pointing rotated + # 180 degrees about the boresight (CMATRIX_ROTATION) into the oops-frame + # convention. The *_SPICE SpiceFrames above stay registered as the + # spice-convention pointing (exactly what cspyce.pxform returns). + # override=True so a rebuild after ISS.reset() refreshes them to the + # current pointing. nac_frame = oops.frame.Cmatrix(CMATRIX_ROTATION, nac_frame_spice, frame_id='CASSINI_ISS_NAC', override=True) @@ -555,8 +532,7 @@ def spice_from_oops(cmatrix): #=========================================================================== @staticmethod - def set_cmatrix(cmatrix, camera, frame_id=None, map_other_camera=False, - time=None): + def set_cmatrix(cmatrix, camera, frame_id=None): """Build an oops observation frame from a custom spice-frame C-matrix and return the frame to attach to the observation. @@ -573,36 +549,20 @@ def set_cmatrix(cmatrix, camera, frame_id=None, map_other_camera=False, cmatrix an oops.Matrix3 (or 3x3 array) for the named camera, in the spice-frame convention. camera 'NAC' or 'WAC', the camera the cmatrix applies to. - frame_id when map_other_camera is False: None (default) returns a - fresh unregistered frame owned by the observation, so - loading other images never disturbs its pointing; - otherwise the frame is registered under this frame ID (a - shared, registered frame). - map_other_camera when True, also derive and register the other - camera's frame from the fixed inter-camera rotation (uses - SPICE); frame_id is ignored. - time the observation time (TDB), required to evaluate the - inter-camera rotation when map_other_camera is True. + frame_id None (default) returns a fresh unregistered frame owned + by the observation, so loading other images never + disturbs its pointing; otherwise the frame is registered + under this frame ID (a shared, registered frame). Return: the frame to attach to the observation: a Cmatrix frame object (unregistered default case) or a registered frame - ID (frame_id given, or map_other_camera). + ID (frame_id given). """ # Convert the spice-frame C-matrix into the oops-frame observation-frame # attitude by applying the fixed 180-degree instrument rotation. attitude = ISS.oops_from_spice(cmatrix) - if map_other_camera: - # Build the SPICE-derived frames first (so the dedicated *_SPICE - # frames exist and the label camera starts from SPICE), then override - # the label camera's global frame and map that pointing to the other - # camera. - ISS.define_camera_frames() - oops.frame.Cmatrix(attitude, frame_id='CASSINI_ISS_' + camera, - override=True) - return ISS.map_other_camera(camera, time) - if frame_id is not None: # Register a single, shared custom frame; global frames untouched. oops.frame.Cmatrix(attitude, frame_id=frame_id) @@ -613,57 +573,6 @@ def set_cmatrix(cmatrix, camera, frame_id=None, map_other_camera=False, # overwritten by any other image's pointing. return oops.frame.Cmatrix(attitude, frame_id=None) - #=========================================================================== - @staticmethod - def map_other_camera(camera, time): - """Register the other camera's frame from the label camera's current - pointing and the fixed inter-camera rotation, and return the label - camera's frame ID. - - Takes whatever pointing the label camera's global frame currently holds - (SPICE-derived, or a custom C-matrix already installed by set_cmatrix) - and maps it to the co-mounted camera, so the two stay consistent. This - overrides the global camera frames. - - The label camera's global frame (CASSINI_ISS_) and the dedicated - *_SPICE frames must already be defined; ISS.define_camera_frames() (or - set_cmatrix() with map_other_camera=True) does this. - - Input: - camera 'NAC' or 'WAC', the label camera whose pointing is mapped. - time the observation time (TDB), used to evaluate the - inter-camera rotation and read the label pointing. - - Return: 'CASSINI_ISS_', the label camera's frame ID. - """ - - # rel = M_other<-label, the fixed rotation read from the dedicated - # *_SPICE frames at the observation time. These are used (rather than - # CASSINI_ISS_NAC/WAC) because a set_cmatrix override may have replaced - # the CASSINI_ISS_ frames with a custom C-matrix, which would - # corrupt the inter-camera rotation. - other = 'WAC' if camera == 'NAC' else 'NAC' - label_wf = oops.frame.Frame.as_wayframe('CASSINI_ISS_' + camera - + '_SPICE') - other_wf = oops.frame.Frame.as_wayframe('CASSINI_ISS_' + other - + '_SPICE') - rel = other_wf.wrt(label_wf).transform_at_time(time).matrix - - # The label camera's current pointing (SPICE or a custom C-matrix). - label = oops.frame.Frame.as_wayframe('CASSINI_ISS_' + camera) \ - .wrt(oops.frame.Frame.J2000).transform_at_time(time).matrix - - oops.frame.Cmatrix(rel * label, - frame_id='CASSINI_ISS_' + other, override=True) - - # The global camera frames now hold this observation's custom pointing. - # Force define_camera_frames() to rebuild them from SPICE on the next - # plain load; otherwise its quick-exit would leak this pointing into the - # next image. - ISS.frames_defined = False - - return 'CASSINI_ISS_' + camera - #=========================================================================== @staticmethod def reset(): diff --git a/tests/hosts/cassini/iss/test_iss.py b/tests/hosts/cassini/iss/test_iss.py index d7880983..c3800325 100644 --- a/tests/hosts/cassini/iss/test_iss.py +++ b/tests/hosts/cassini/iss/test_iss.py @@ -19,8 +19,8 @@ class Test_Cassini_ISS_Cmatrix(unittest.TestCase): """Tests for the custom C-matrix support in hosts/cassini/iss.py - (from_file's cmatrix/frame_id/map_other_camera arguments and - ISS.set_cmatrix), plus the generic Observation.get_cmatrix() getter. + (from_file's cmatrix/frame_id arguments and ISS.set_cmatrix), plus the + generic Observation.get_cmatrix() getter. The `cmatrix` accepted by from_file is the SPICE camera-frame C-matrix (the J2000 -> CASSINI_ISS_ rotation, as returned by cspyce.pxform). The @@ -47,13 +47,13 @@ def tearDown(self): #=========================================================================== def _spice_cmatrix(self, camera, time): """The recorded SPICE camera-frame C-matrix for the given camera and - time. CASSINI_ISS__FLIPPED is the SpiceFrame wrapping SPICE's + time. CASSINI_ISS__SPICE is the SpiceFrame wrapping SPICE's CASSINI_ISS_, so its J2000 attitude is exactly cspyce.pxform('J2000', 'CASSINI_ISS_', time). Requires a prior - plain load (define_camera_frames) so the *_FLIPPED frame is registered. + plain load (define_camera_frames) so the *_SPICE frame is registered. """ - return (Frame.as_wayframe('CASSINI_ISS_' + camera + '_FLIPPED') + return (Frame.as_wayframe('CASSINI_ISS_' + camera + '_SPICE') .wrt(Frame.J2000).transform_at_time(time).matrix) #=========================================================================== @@ -157,91 +157,8 @@ def test_explicit_frame_id_leaves_global_untouched(self): baseline.get_cmatrix().vals)) #=========================================================================== - def test_map_other_camera(self): - """map_other_camera=True derives the co-mounted camera's frame from - the fixed, SPICE-derived inter-camera rotation, preserving it under a - custom C-matrix.""" - - baseline = from_file(self.filespec) - camera = baseline.dict['INSTRUMENT_ID'][3:] + 'C' - other = 'NAC' if camera == 'WAC' else 'WAC' - spice0 = self._spice_cmatrix(camera, baseline.tstart) - - baseline_rel = (Frame.as_wayframe('CASSINI_ISS_' + other) - .wrt(Frame.as_wayframe('CASSINI_ISS_' + camera)) - .transform_at_time(baseline.tstart).matrix) - - custom = _PERTURBATION * spice0 - obs = from_file(self.filespec, cmatrix=custom, map_other_camera=True) - - self.assertEqual(obs.frame.frame_id, 'CASSINI_ISS_' + camera) - - # The fixed inter-camera rotation is preserved under the override. - new_rel = (Frame.as_wayframe('CASSINI_ISS_' + other) - .wrt(Frame.as_wayframe('CASSINI_ISS_' + camera)) - .transform_at_time(baseline.tstart).matrix) - self.assertTrue(np.allclose(new_rel.vals, baseline_rel.vals)) - - # ...and the other camera's absolute pointing was updated accordingly. - other_matrix = (Frame.as_wayframe('CASSINI_ISS_' + other) - .wrt(Frame.J2000) - .transform_at_time(baseline.tstart).matrix) - self.assertTrue(np.allclose(other_matrix.vals, - (baseline_rel * CMATRIX_ROTATION * custom).vals)) - - #=========================================================================== - def test_map_reclaims_global_on_plain_load(self): - """A mapped custom load overrides the global camera frames; a later - plain load must rebuild them from SPICE rather than inheriting the - custom pointing.""" - - baseline = from_file(self.filespec) - m0 = baseline.get_cmatrix() - camera = baseline.dict['INSTRUMENT_ID'][3:] + 'C' - spice0 = self._spice_cmatrix(camera, baseline.tstart) - - _ = from_file(self.filespec, cmatrix=_PERTURBATION * spice0, - map_other_camera=True) - - plain = from_file(self.filespec) - self.assertTrue(np.allclose(plain.get_cmatrix().vals, m0.vals)) - - #=========================================================================== - def test_map_after_map_override(self): - """A mapped load followed by another mapped load must still derive the - inter-camera rotation from the dedicated *_SPICE frames, not from the - CASSINI_ISS_ frames a prior mapped load overrode.""" - - baseline = from_file(self.filespec) - camera = baseline.dict['INSTRUMENT_ID'][3:] + 'C' - other = 'NAC' if camera == 'WAC' else 'WAC' - spice0 = self._spice_cmatrix(camera, baseline.tstart) - - # The true, SPICE-derived inter-camera rotation. - baseline_rel = (Frame.as_wayframe('CASSINI_ISS_' + other) - .wrt(Frame.as_wayframe('CASSINI_ISS_' + camera)) - .transform_at_time(baseline.tstart).matrix) - - # First mapped load with an unrelated pointing overrides both global - # camera frames. - bogus = _PERTURBATION * _PERTURBATION * spice0 - _ = from_file(self.filespec, cmatrix=bogus, map_other_camera=True) - - # Second mapped load: rel must come from the dedicated *_SPICE frames, - # unaffected by the override above. - custom = _PERTURBATION * spice0 - _ = from_file(self.filespec, cmatrix=custom, map_other_camera=True) - - other_matrix = (Frame.as_wayframe('CASSINI_ISS_' + other) - .wrt(Frame.J2000) - .transform_at_time(baseline.tstart).matrix) - self.assertTrue(np.allclose(other_matrix.vals, - (baseline_rel * CMATRIX_ROTATION * custom).vals)) - - #=========================================================================== - def test_no_ck_loaded_without_mapping(self): - """A custom cmatrix without map_other_camera never loads a CK (no - SPICE pointing dependency).""" + def test_custom_cmatrix_loads_no_ck(self): + """A custom cmatrix never loads a CK (no SPICE pointing dependency).""" self.assertFalse(np.any(Cassini.CK_LOADED)) @@ -250,37 +167,13 @@ def test_no_ck_loaded_without_mapping(self): self.assertFalse(np.any(Cassini.CK_LOADED)) #=========================================================================== - def test_mapping_or_spice_pointing_loads_ck(self): - """SPICE pointing (no cmatrix), and map_other_camera=True, each load - a CK.""" + def test_spice_pointing_loads_ck(self): + """SPICE pointing (no cmatrix) loads a CK.""" self.assertFalse(np.any(Cassini.CK_LOADED)) _ = from_file(self.filespec) self.assertTrue(np.any(Cassini.CK_LOADED)) - ISS.reset() - self.assertFalse(np.any(Cassini.CK_LOADED)) - _ = from_file(self.filespec, cmatrix=oops.Matrix3(np.eye(3)), - map_other_camera=True) - self.assertTrue(np.any(Cassini.CK_LOADED)) - - #=========================================================================== - def test_reset_restores_spice_pointing(self): - """After ISS.reset(), from_file() without cmatrix restores the - SPICE-derived pointing even if a custom cmatrix was used earlier.""" - - baseline = from_file(self.filespec) - m0 = baseline.get_cmatrix() - camera = baseline.dict['INSTRUMENT_ID'][3:] + 'C' - spice0 = self._spice_cmatrix(camera, baseline.tstart) - - _ = from_file(self.filespec, cmatrix=_PERTURBATION * spice0, - map_other_camera=True) - - ISS.reset() - restored = from_file(self.filespec) - self.assertTrue(np.allclose(restored.get_cmatrix().vals, m0.vals)) - ############################################ if __name__ == '__main__': unittest.main(verbosity=2) From 0d226dff05685c767fdcc27f148e6db85a52ee03 Mon Sep 17 00:00:00 2001 From: Joseph Spitale Date: Wed, 12 Aug 2026 16:24:17 -0700 Subject: [PATCH 13/22] Delete ISS.set_cmatrix in favor of Observation.set_cmatrix Observation.set_cmatrix now attaches the frame itself (assigns self.frame) instead of returning it. from_file builds the Snapshot with a placeholder J2000 frame when a custom C-matrix is given, then applies the pointing through the generic method once the host subfield is in place; no CKs are loaded. Co-Authored-By: Claude Fable 5 --- oops/hosts/cassini/iss.py | 55 +++++------------------------ oops/observation/observation_.py | 31 ++++++++-------- tests/hosts/cassini/iss/test_iss.py | 4 +-- 3 files changed, 24 insertions(+), 66 deletions(-) diff --git a/oops/hosts/cassini/iss.py b/oops/hosts/cassini/iss.py index 6664cdc5..abb7dc7e 100755 --- a/oops/hosts/cassini/iss.py +++ b/oops/hosts/cassini/iss.py @@ -101,11 +101,11 @@ def from_file(filespec, fast_distortion=True, if cmatrix is None: Cassini.load_cks(tstart, tstart + texp) - # Determine the observation frame, applying a custom C-matrix if given. - # ISS.set_cmatrix builds the frame and returns the one to attach to this - # observation; SPICE pointing is circumvented. + # Determine the observation frame. With a custom C-matrix, SPICE pointing + # is circumvented: the Snapshot is built with a placeholder J2000 frame + # (no CKs needed) and Observation.set_cmatrix replaces it below. if cmatrix is not None: - frame = ISS.set_cmatrix(cmatrix, camera, frame_id=frame_id) + frame = 'J2000' else: ISS.define_camera_frames() # use the SPICE-derived pointing frame = 'CASSINI_ISS_' + camera @@ -125,6 +125,10 @@ def from_file(filespec, fast_distortion=True, filter2 = filter2, gain_mode = gain_mode) + # Apply the custom pointing, replacing the placeholder frame. + if cmatrix is not None: + result.set_cmatrix(cmatrix, frame_id=frame_id) + result.insert_subfield('spice_kernels', Cassini.used_kernels(result.time, 'iss', return_all_planets)) @@ -530,49 +534,6 @@ def spice_from_oops(cmatrix): return CMATRIX_ROTATION * oops.Matrix3.as_matrix3(cmatrix) - #=========================================================================== - @staticmethod - def set_cmatrix(cmatrix, camera, frame_id=None): - """Build an oops observation frame from a custom spice-frame C-matrix - and return the frame to attach to the observation. - - ISS.initialize() must have been called first. The C-matrix is the - spice-frame pointing (the rotation from J2000 into SPICE's - CASSINI_ISS_ frame, as returned by cspyce.pxform() or recorded in - a CK; z along the line of sight, x left, y up). The oops-frame - observation frame is that frame rotated 180 degrees about the boresight - (see CMATRIX_ROTATION; z along the line of sight, x right, y down); this rotation - is applied here, at the boundary, so a recorded C-matrix reproduces the - observation's pointing. - - Input: - cmatrix an oops.Matrix3 (or 3x3 array) for the named camera, in - the spice-frame convention. - camera 'NAC' or 'WAC', the camera the cmatrix applies to. - frame_id None (default) returns a fresh unregistered frame owned - by the observation, so loading other images never - disturbs its pointing; otherwise the frame is registered - under this frame ID (a shared, registered frame). - - Return: the frame to attach to the observation: a Cmatrix frame - object (unregistered default case) or a registered frame - ID (frame_id given). - """ - - # Convert the spice-frame C-matrix into the oops-frame observation-frame - # attitude by applying the fixed 180-degree instrument rotation. - attitude = ISS.oops_from_spice(cmatrix) - - if frame_id is not None: - # Register a single, shared custom frame; global frames untouched. - oops.frame.Cmatrix(attitude, frame_id=frame_id) - return frame_id - - # Default: a fresh unregistered frame owned by this observation. It never - # enters the global registry, so it neither collides with nor is - # overwritten by any other image's pointing. - return oops.frame.Cmatrix(attitude, frame_id=None) - #=========================================================================== @staticmethod def reset(): diff --git a/oops/observation/observation_.py b/oops/observation/observation_.py index 52e03895..9adf272a 100755 --- a/oops/observation/observation_.py +++ b/oops/observation/observation_.py @@ -488,8 +488,7 @@ def get_cmatrix(self, uv=None, time=None): #=========================================================================== def set_cmatrix(self, cmatrix, frame_id=None): - """Build an oops observation frame from a spice-convention C-matrix and - return the frame to attach to the observation. + """Set this observation's frame from a spice-convention C-matrix. This is the inverse of get_cmatrix(): it takes the spice-convention C-matrix (for ISS, the spice-frame pointing: the rotation from J2000 @@ -499,19 +498,16 @@ def set_cmatrix(self, cmatrix, frame_id=None): (the 'host' subfield's CMATRIX_ROTATION attribute; z along the line of sight, x right, y down). Because that rotation is its own inverse, the same product recovers the observation attitude from the C-matrix that - get_cmatrix() applied to produce it. + get_cmatrix() applied to produce it. The resulting frame replaces + self.frame. Input: cmatrix an oops.Matrix3 (or 3x3 array) in the spice-convention C-matrix, as returned by get_cmatrix(). - frame_id None (default) returns a fresh unregistered frame owned by - the observation, so loading other images never disturbs - its pointing; otherwise the frame is registered under this - frame ID (a shared, registered frame). - - Return: the frame to attach to the observation: a Cmatrix frame - object (unregistered default case) or a registered frame - ID (frame_id given). + frame_id None (default) attaches a fresh unregistered frame owned + by the observation, so loading other images never + disturbs its pointing; otherwise the frame is registered + under this frame ID (a shared, registered frame). """ # Convert the spice-convention C-matrix into the oops observation-frame @@ -520,13 +516,14 @@ def set_cmatrix(self, cmatrix, frame_id=None): if frame_id is not None: # Register a single, shared custom frame; global frames untouched. - Cmatrix(attitude, frame_id=frame_id) - return frame_id + frame = Cmatrix(attitude, frame_id=frame_id) + else: + # A fresh unregistered frame owned by this observation. It never + # enters the global registry, so it neither collides with nor is + # overwritten by any other image's pointing. + frame = Cmatrix(attitude, frame_id=None) - # Default: a fresh unregistered frame owned by this observation. It never - # enters the global registry, so it neither collides with nor is - # overwritten by any other image's pointing. - return Cmatrix(attitude, frame_id=None) + self.frame = Frame.as_wayframe(frame) #=========================================================================== def meshgrid(self, origin=None, undersample=1, oversample=1, limit=None, diff --git a/tests/hosts/cassini/iss/test_iss.py b/tests/hosts/cassini/iss/test_iss.py index c3800325..d6f90d98 100644 --- a/tests/hosts/cassini/iss/test_iss.py +++ b/tests/hosts/cassini/iss/test_iss.py @@ -19,8 +19,8 @@ class Test_Cassini_ISS_Cmatrix(unittest.TestCase): """Tests for the custom C-matrix support in hosts/cassini/iss.py - (from_file's cmatrix/frame_id arguments and ISS.set_cmatrix), plus the - generic Observation.get_cmatrix() getter. + (from_file's cmatrix/frame_id arguments), plus the generic + Observation.get_cmatrix() / Observation.set_cmatrix() methods. The `cmatrix` accepted by from_file is the SPICE camera-frame C-matrix (the J2000 -> CASSINI_ISS_ rotation, as returned by cspyce.pxform). The From 202305ba570baa36028598547f869682f3d0e377 Mon Sep 17 00:00:00 2001 From: Joseph Spitale Date: Wed, 12 Aug 2026 16:29:05 -0700 Subject: [PATCH 14/22] Remove unused ISS.oops_from_spice/spice_from_oops conversion helpers Co-Authored-By: Claude Fable 5 --- oops/hosts/cassini/iss.py | 40 ----------------------------- tests/hosts/cassini/iss/test_iss.py | 14 +--------- 2 files changed, 1 insertion(+), 53 deletions(-) diff --git a/oops/hosts/cassini/iss.py b/oops/hosts/cassini/iss.py index abb7dc7e..b1be63a5 100755 --- a/oops/hosts/cassini/iss.py +++ b/oops/hosts/cassini/iss.py @@ -494,46 +494,6 @@ def define_camera_frames(): ISS.frames_defined = True - #=========================================================================== - @staticmethod - def oops_from_spice(cmatrix): - """Convert a spice-frame C-matrix into the oops-frame attitude. - - The spice-frame is the SPICE camera-frame C-matrix: the rotation from - J2000 into SPICE's CASSINI_ISS_ frame, as returned by - cspyce.pxform() or recorded in a CK (z along the line of sight, x left, - y up). The oops-frame is the oops observation frame, that SPICE frame - rotated 180 degrees about the boresight (z along the line of sight, x - right, y down). - - Input: - cmatrix an oops.Matrix3 (or 3x3 array) in the spice-frame - convention. - - Return: an oops.Matrix3 in the oops-frame convention. - """ - - return CMATRIX_ROTATION * oops.Matrix3.as_matrix3(cmatrix) - - #=========================================================================== - @staticmethod - def spice_from_oops(cmatrix): - """Convert an oops-frame attitude into a spice-frame C-matrix. - - The inverse of oops_from_spice(); see that method for the two - conventions and their axes. Because the frames differ by a 180-degree - rotation (its own inverse), this applies the same rotation. - - Input: - cmatrix an oops.Matrix3 (or 3x3 array) in the oops-frame - convention (z along the line of sight, x right, y down). - - Return: an oops.Matrix3 in the spice-frame convention (z along - the line of sight, x left, y up). - """ - - return CMATRIX_ROTATION * oops.Matrix3.as_matrix3(cmatrix) - #=========================================================================== @staticmethod def reset(): diff --git a/tests/hosts/cassini/iss/test_iss.py b/tests/hosts/cassini/iss/test_iss.py index d6f90d98..4ba0c393 100644 --- a/tests/hosts/cassini/iss/test_iss.py +++ b/tests/hosts/cassini/iss/test_iss.py @@ -9,7 +9,7 @@ from oops.body import Body from oops.frame import Frame from oops.hosts.cassini import Cassini -from oops.hosts.cassini.iss import ISS, from_file, CMATRIX_ROTATION +from oops.hosts.cassini.iss import ISS, from_file from oops.unittester_support import TEST_DATA_PREFIX # A rotation distinguishable from any SPICE-derived pointing, used to build @@ -56,18 +56,6 @@ def _spice_cmatrix(self, camera, time): return (Frame.as_wayframe('CASSINI_ISS_' + camera + '_SPICE') .wrt(Frame.J2000).transform_at_time(time).matrix) - #=========================================================================== - def test_convention_conversions(self): - """ISS.oops_from_spice / spice_from_oops convert between the two - conventions and are mutual inverses.""" - - # Mutual inverses, for an arbitrary rotation. - m = _PERTURBATION * CMATRIX_ROTATION * _PERTURBATION - self.assertTrue(np.allclose(m.vals, - ISS.spice_from_oops(ISS.oops_from_spice(m)).vals)) - self.assertTrue(np.allclose(m.vals, - ISS.oops_from_spice(ISS.spice_from_oops(m)).vals)) - #=========================================================================== def test_get_cmatrix_inverts_set(self): """get_cmatrix returns the SPICE-convention C-matrix (matching From 87c6062516ddd2c92bb7d96f7e4aaf9590109aff Mon Sep 17 00:00:00 2001 From: Joseph Spitale Date: Thu, 13 Aug 2026 13:45:21 -0700 Subject: [PATCH 15/22] Fix custom C-matrix mechanism: single-attribute rotation, override on frame_id reuse, informative errors get_cmatrix/set_cmatrix now derive the spice-frame<->oops-frame inverse rotation by transposing the host's single CMATRIX_ROTATION attribute, instead of requiring hosts to expose a redundant CMATRIX_ROTATION_INV that could be defined inconsistently. Re-using a frame_id in set_cmatrix now passes override=True so the new pointing cleanly replaces the primary frame definition instead of leaving a stale one behind (Frame.register's secondary-definition path). Both methods now raise a clear ValueError when the observation's host doesn't define CMATRIX_ROTATION, rather than a bare AttributeError. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01WWmXpTiygmBh786ED6GigN --- oops/hosts/cassini/iss.py | 15 ++++--- oops/observation/observation_.py | 70 ++++++++++++++++------------- tests/hosts/cassini/iss/test_iss.py | 55 +++++++++++++++++++++++ 3 files changed, 103 insertions(+), 37 deletions(-) diff --git a/oops/hosts/cassini/iss.py b/oops/hosts/cassini/iss.py index b1be63a5..e1e1d90c 100755 --- a/oops/hosts/cassini/iss.py +++ b/oops/hosts/cassini/iss.py @@ -14,7 +14,7 @@ from oops.hosts.cassini import Cassini # There are two C-matrix conventions here, related by CMATRIX_ROTATION (a 180-degree spin -# about the boresight; it is its own inverse): +# about the boresight): # * spice-frame: the pointing straight from SPICE (a CK or cspyce.pxform), i.e. # the J2000 -> CASSINI_ISS_ rotation. Axes: z along the line of # sight, x to the left, y up. @@ -22,7 +22,9 @@ # oops convention: z along the line of sight, x to the right, y down. # oops-frame = CMATRIX_ROTATION * spice-frame. The instrument's internal coordinate system # matches the oops-frame, so a recorded (spice-frame) C-matrix is rotated by -# CMATRIX_ROTATION at the boundary to build the observation frame. +# CMATRIX_ROTATION at the boundary to build the observation frame. The +# generic Observation.get_cmatrix()/set_cmatrix() methods derive the inverse +# (spice-frame from oops-frame) by transposing CMATRIX_ROTATION. CMATRIX_ROTATION = oops.Matrix3([[-1,0,0],[0,-1,0],[0,0,1]]) ################################################################################ @@ -235,9 +237,12 @@ class ISS(object): frames_defined = False offset_wac = False - # Exposed on the class so the generic Observation.get_cmatrix() can read the - # host's oops-frame -> spice-frame convention rotation via the observation's - # `host` subfield. See the module-level CMATRIX_ROTATION definition above. + # Exposed on the class so the generic Observation.set_cmatrix() and + # get_cmatrix() can read the host's convention rotation via the + # observation's `host` subfield: CMATRIX_ROTATION converts spice-frame to + # oops-frame. Any host supporting the generic cmatrix methods must expose + # this single attribute; the inverse (oops-frame to spice-frame) is + # derived by transposition. See the module-level definition above. CMATRIX_ROTATION = CMATRIX_ROTATION # Create a master version of the NAC and WAC distortion models from diff --git a/oops/observation/observation_.py b/oops/observation/observation_.py index 9adf272a..f79fca78 100755 --- a/oops/observation/observation_.py +++ b/oops/observation/observation_.py @@ -457,16 +457,13 @@ def get_cmatrix(self, uv=None, time=None): """The spice-convention C-matrix of this observation. This derives the observation's pointing from its own frame and the fixed - convention rotation exposed by its host (the 'host' subfield's CMATRIX_ROTATION - attribute): it evaluates the oops observation-frame attitude and applies - the host rotation. For ISS the result is the spice-frame C-matrix: the - rotation from J2000 into SPICE's CASSINI_ISS_ frame, as - cspyce.pxform() returns, with z along the line of sight, x left, y up. - Note this is NOT the oops-frame convention (z along the line of sight, x - right, y down) of the observation frame itself; the two differ by that - 180-degree rotation about the boresight. Because it is derived from - self.frame at call time, the result reflects this observation's own - pointing and is unaffected by other loads. + convention rotation exposed by its host: it evaluates the oops + observation-frame attitude and applies the transpose of the host's + spice-frame -> oops-frame rotation (the 'host' subfield's + CMATRIX_ROTATION attribute, the same rotation applied by + set_cmatrix()). The result is the spice-frame C-matrix, as returned by + cspyce.pxform(). Note this is NOT the oops-frame convention (z along + the line of sight, x right, y down) of the observation frame itself. Input: uv a (u,v) pixel location, used only to select a time when @@ -479,49 +476,58 @@ def get_cmatrix(self, uv=None, time=None): the spice-frame C-matrix). """ + try: + rotation = self.host.CMATRIX_ROTATION + except AttributeError: + raise ValueError('get_cmatrix requires an observation whose host ' + 'class defines CMATRIX_ROTATION (the spice-frame ' + '-> oops-frame convention rotation)') + if time is None: uv = self.fov.uv_shape / 2. if uv is None else Pair.as_pair(uv) time = self.midtime_at_uv(uv) oops_attitude = self.frame.wrt(Frame.J2000).transform_at_time(time).matrix - return self.host.CMATRIX_ROTATION * oops_attitude + return rotation.transpose() * oops_attitude #=========================================================================== def set_cmatrix(self, cmatrix, frame_id=None): """Set this observation's frame from a spice-convention C-matrix. - This is the inverse of get_cmatrix(): it takes the spice-convention - C-matrix (for ISS, the spice-frame pointing: the rotation from J2000 - into SPICE's CASSINI_ISS_ frame, with z along the line of sight, - x left, y up) and converts it into the oops observation-frame attitude by - applying the fixed convention rotation exposed by the observation's host - (the 'host' subfield's CMATRIX_ROTATION attribute; z along the line of - sight, x right, y down). Because that rotation is its own inverse, the - same product recovers the observation attitude from the C-matrix that - get_cmatrix() applied to produce it. The resulting frame replaces - self.frame. + Converts the spice-convention C-matrix into the oops observation-frame + attitude by applying the host's spice-frame -> oops-frame rotation (the + 'host' subfield's CMATRIX_ROTATION attribute; get_cmatrix() applies its + transpose). The resulting frame replaces self.frame. Input: - cmatrix an oops.Matrix3 (or 3x3 array) in the spice-convention + cmatrix an oops.Matrix3 (or 3x3 array) giving the spice-convention C-matrix, as returned by get_cmatrix(). frame_id None (default) attaches a fresh unregistered frame owned by the observation, so loading other images never disturbs its pointing; otherwise the frame is registered - under this frame ID (a shared, registered frame). + under this frame ID (a shared, registered frame). Note + that re-using a frame_id re-points every observation + that shares that registered frame; that sharing is the + point of giving it an ID, but it is worth stating + explicitly. """ + try: + rotation = self.host.CMATRIX_ROTATION + except AttributeError: + raise ValueError('set_cmatrix requires an observation whose host ' + 'class defines CMATRIX_ROTATION (the spice-frame ' + '-> oops-frame convention rotation)') + # Convert the spice-convention C-matrix into the oops observation-frame # attitude by applying the fixed instrument convention rotation. - attitude = self.host.CMATRIX_ROTATION * Matrix3.as_matrix3(cmatrix) + attitude = rotation * Matrix3.as_matrix3(cmatrix) - if frame_id is not None: - # Register a single, shared custom frame; global frames untouched. - frame = Cmatrix(attitude, frame_id=frame_id) - else: - # A fresh unregistered frame owned by this observation. It never - # enters the global registry, so it neither collides with nor is - # overwritten by any other image's pointing. - frame = Cmatrix(attitude, frame_id=None) + # frame_id=None attaches a fresh unregistered frame owned by this + # observation, isolated from the global registry. A given frame_id + # registers (or re-registers, with override=True) a single shared + # frame under that ID, cleanly replacing any prior primary definition. + frame = Cmatrix(attitude, frame_id=frame_id, override=(frame_id is not None)) self.frame = Frame.as_wayframe(frame) diff --git a/tests/hosts/cassini/iss/test_iss.py b/tests/hosts/cassini/iss/test_iss.py index 4ba0c393..50b90561 100644 --- a/tests/hosts/cassini/iss/test_iss.py +++ b/tests/hosts/cassini/iss/test_iss.py @@ -144,6 +144,61 @@ def test_explicit_frame_id_leaves_global_untouched(self): self.assertTrue(np.allclose(unaffected.get_cmatrix().vals, baseline.get_cmatrix().vals)) + #=========================================================================== + def test_reused_frame_id_replaces_primary(self): + """Re-using a frame_id re-points every observation sharing that + registered frame; the primary registry definition is cleanly replaced, + not shadowed by a stale entry (Frame.register's secondary-definition + path).""" + + baseline = from_file(self.filespec) + camera = baseline.dict['INSTRUMENT_ID'][3:] + 'C' + spice0 = self._spice_cmatrix(camera, baseline.tstart) + + custom1 = _PERTURBATION * spice0 + obs1 = from_file(self.filespec, cmatrix=custom1, + frame_id='TEST_REUSED_FRAME') + self.assertTrue(np.allclose(obs1.get_cmatrix().vals, custom1.vals)) + + custom2 = _PERTURBATION * _PERTURBATION * spice0 + obs2 = from_file(self.filespec, cmatrix=custom2, + frame_id='TEST_REUSED_FRAME') + self.assertTrue(np.allclose(obs2.get_cmatrix().vals, custom2.vals)) + + # The primary registry definition must reflect the new pointing, not + # a stale first definition left behind by a secondary registration. + primary = Frame.as_primary_frame('TEST_REUSED_FRAME') + attitude = primary.wrt(Frame.J2000).transform_at_time(baseline.tstart).matrix + recovered = ISS.CMATRIX_ROTATION.transpose() * attitude + self.assertTrue(np.allclose(recovered.vals, custom2.vals)) + self.assertFalse(np.allclose(recovered.vals, custom1.vals)) + + #=========================================================================== + def test_frame_id_without_cmatrix_raises(self): + """frame_id is only meaningful together with cmatrix; passing it alone + must raise rather than being silently ignored.""" + + with self.assertRaises(ValueError): + from_file(self.filespec, frame_id='TEST_FRAME_ID_ALONE') + + #=========================================================================== + def test_missing_host_contract_raises(self): + """set_cmatrix/get_cmatrix on an observation with no `host` subfield + raise an informative ValueError rather than a bare AttributeError.""" + + baseline = from_file(self.filespec) + bare = oops.obs.Snapshot(('v', 'u'), baseline.tstart, baseline.texp, + baseline.fov, path=baseline.path, + frame=baseline.frame) + + self.assertFalse(hasattr(bare, 'host')) + + with self.assertRaises(ValueError): + bare.get_cmatrix() + + with self.assertRaises(ValueError): + bare.set_cmatrix(oops.Matrix3(np.eye(3))) + #=========================================================================== def test_custom_cmatrix_loads_no_ck(self): """A custom cmatrix never loads a CK (no SPICE pointing dependency).""" From 3ddb0708dc543bff035da4bd5df51a5b21e79cde Mon Sep 17 00:00:00 2001 From: Joseph Spitale Date: Thu, 13 Aug 2026 13:45:51 -0700 Subject: [PATCH 16/22] Reject from_file frame_id without cmatrix frame_id was only consulted when cmatrix was also given; passing it alone was silently ignored rather than raising. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01WWmXpTiygmBh786ED6GigN --- oops/hosts/cassini/iss.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/oops/hosts/cassini/iss.py b/oops/hosts/cassini/iss.py index e1e1d90c..56140bb0 100755 --- a/oops/hosts/cassini/iss.py +++ b/oops/hosts/cassini/iss.py @@ -59,9 +59,12 @@ def from_file(filespec, fast_distortion=True, this frame ID and use only that (shared, registered) frame. None (default) gives the observation its own unregistered frame, so loading other images never - disturbs this one's pointing. + disturbs this one's pointing. Requires cmatrix. """ + if cmatrix is None and frame_id is not None: + raise ValueError('frame_id requires a cmatrix') + ISS.initialize() # Define everything the first time through; use defaults # unless initialize() is called explicitly. From dd7f8832605be021992465addd737e5e524abf7e Mon Sep 17 00:00:00 2001 From: Joseph Spitale Date: Thu, 13 Aug 2026 13:46:07 -0700 Subject: [PATCH 17/22] Exclude CK kernels from spice_kernels on custom-cmatrix loads Cassini.used_kernels() gained a ck keyword; from_file passes ck=(cmatrix is None). Without this, a custom-cmatrix observation's spice_kernels could list CK basenames it never used -- e.g. the gapfill CKs Cassini.initialize() furnishes unconditionally, or a CK furnished earlier in the session by a plain SPICE-pointed load. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01WWmXpTiygmBh786ED6GigN --- oops/hosts/cassini/__init__.py | 22 +++++++++++++++++++--- oops/hosts/cassini/iss.py | 7 ++++++- tests/hosts/cassini/iss/test_iss.py | 17 +++++++++++++++++ 3 files changed, 42 insertions(+), 4 deletions(-) diff --git a/oops/hosts/cassini/__init__.py b/oops/hosts/cassini/__init__.py index 692ecdf1..4cec6c07 100755 --- a/oops/hosts/cassini/__init__.py +++ b/oops/hosts/cassini/__init__.py @@ -350,9 +350,21 @@ def spice_frames_kernel(asof=None): #=========================================================================== @staticmethod - def used_kernels(time, inst, return_all_planets=False): + def used_kernels(time, inst, return_all_planets=False, ck=True): """The list of kernels associated with a Cassini observation at a selected range of times. + + Input: + time a (start, stop) tuple of times in seconds TDB. + inst the instrument name, e.g., 'iss'. + return_all_planets Include kernels for all planets not just + Jupiter or Saturn. + ck True (default) to include CK (pointing) + kernels; False to exclude them, e.g. for an + observation whose pointing came from a custom + cmatrix rather than SPICE, where any + furnished CK is unrelated to how the + observation was actually pointed. """ if return_all_planets: bodies = [1, 199, 2, 299, 3, 399, 4, 499, 5, 599, 6, 699, @@ -367,7 +379,11 @@ def used_kernels(time, inst, return_all_planets=False): else: bodies = [5, 599] + Body.JUPITER_MOONS_LOADED - return spicedb.used_basenames(time=time, inst=inst, sc=-82, - bodies=bodies) + types = None + if not ck: + types = [t for t in spicedb.KERNEL_TYPE_SORT_ORDER if t != 'CK'] + + return spicedb.used_basenames(types=types, time=time, inst=inst, + sc=-82, bodies=bodies) ################################################################################ diff --git a/oops/hosts/cassini/iss.py b/oops/hosts/cassini/iss.py index 56140bb0..589fc773 100755 --- a/oops/hosts/cassini/iss.py +++ b/oops/hosts/cassini/iss.py @@ -134,9 +134,14 @@ def from_file(filespec, fast_distortion=True, if cmatrix is not None: result.set_cmatrix(cmatrix, frame_id=frame_id) + # With a custom cmatrix, pointing never came from a CK, so any CK that + # happens to be furnished (e.g. the gapfill CKs loaded unconditionally by + # Cassini.initialize()) is unrelated to this observation and must not be + # reported as used. result.insert_subfield('spice_kernels', Cassini.used_kernels(result.time, 'iss', - return_all_planets)) + return_all_planets, + ck=(cmatrix is None))) result.insert_subfield('filespec', filespec) result.insert_subfield('basename', filespec.name) diff --git a/tests/hosts/cassini/iss/test_iss.py b/tests/hosts/cassini/iss/test_iss.py index 50b90561..a7c9e1b2 100644 --- a/tests/hosts/cassini/iss/test_iss.py +++ b/tests/hosts/cassini/iss/test_iss.py @@ -217,6 +217,23 @@ def test_spice_pointing_loads_ck(self): _ = from_file(self.filespec) self.assertTrue(np.any(Cassini.CK_LOADED)) + #=========================================================================== + def test_custom_cmatrix_reports_no_ck_kernels(self): + """A custom cmatrix's spice_kernels must not report any CK, even one + furnished for an unrelated reason (e.g. the gapfill CKs loaded + unconditionally by Cassini.initialize(), or a CK furnished earlier in + the session by a plain SPICE-pointed load).""" + + # A plain load furnishes and reports at least one CK. + plain = from_file(self.filespec) + self.assertTrue(any(name.endswith('.bc') for name in plain.spice_kernels)) + + # A subsequent custom-cmatrix load must report none, despite CKs + # (including the one just furnished above) being present in the + # kernel pool. + custom = from_file(self.filespec, cmatrix=oops.Matrix3(np.eye(3))) + self.assertFalse(any(name.endswith('.bc') for name in custom.spice_kernels)) + ############################################ if __name__ == '__main__': unittest.main(verbosity=2) From b54fbf2d4a52adb2a28648e0eacfb99dec614efc Mon Sep 17 00:00:00 2001 From: Joseph Spitale Date: Thu, 13 Aug 2026 13:46:18 -0700 Subject: [PATCH 18/22] Fix typo and stale comment in ISS camera-frame setup - "coordinate system" had a double space. - The define_camera_frames comment claimed override=True refreshes frames after ISS.reset(), but neither ISS.reset() nor Cassini.reset() clears the frame registry, so the *_SPICE SpiceFrames (registered without override) leave a stale primary definition after a rebuild. Harmless here since they're used as direct object references, not looked up by ID, but the comment now says so instead of implying a full refresh. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01WWmXpTiygmBh786ED6GigN --- oops/hosts/cassini/iss.py | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/oops/hosts/cassini/iss.py b/oops/hosts/cassini/iss.py index 589fc773..1f765712 100755 --- a/oops/hosts/cassini/iss.py +++ b/oops/hosts/cassini/iss.py @@ -463,7 +463,7 @@ def define_camera_frames(): # Construct a SpiceFrame for each camera # Deal with the fact that the instrument's internal - # coordinate system is rotated 180 degrees (see CMATRIX_ROTATION) + # coordinate system is rotated 180 degrees (see CMATRIX_ROTATION) nac_frame_spice = oops.frame.SpiceFrame('CASSINI_ISS_NAC', frame_id='CASSINI_ISS_NAC_SPICE') wac_frame_spice = oops.frame.SpiceFrame('CASSINI_ISS_WAC', @@ -489,8 +489,12 @@ def define_camera_frames(): # 180 degrees about the boresight (CMATRIX_ROTATION) into the oops-frame # convention. The *_SPICE SpiceFrames above stay registered as the # spice-convention pointing (exactly what cspyce.pxform returns). - # override=True so a rebuild after ISS.reset() refreshes them to the - # current pointing. + # override=True replaces CASSINI_ISS_NAC/_WAC's primary registration on + # each rebuild. Note that neither ISS.reset() nor Cassini.reset() clears + # the frame registry, so the *_SPICE SpiceFrames above (registered + # without override) leave a stale primary definition after a rebuild; + # that's harmless here since nac_frame_spice/wac_frame_spice are used + # directly as object references below, not looked up by ID. nac_frame = oops.frame.Cmatrix(CMATRIX_ROTATION, nac_frame_spice, frame_id='CASSINI_ISS_NAC', override=True) From 6081accd925bb919c2428f5f2afad6910ed64a21 Mon Sep 17 00:00:00 2001 From: Joseph Spitale Date: Mon, 17 Aug 2026 10:49:43 -0700 Subject: [PATCH 19/22] Remove the offset_wac WAC frame-offset option With offset_wac=True, the WAC observation frame carried a Navigation offset on top of the SPICE chain, so get_cmatrix() reported a composite attitude rather than the pxform value its docstring promises, and the meaning of from_file(cmatrix=) depended on a flag set at initialize time. Resolves finding F3 of the rms-oops #201 verification review by removing the option entirely: both camera frames are now built the same way, directly from the SPICE frame and CMATRIX_ROTATION. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_015Wg9zMfz6FNEotebnS1hmm --- oops/hosts/cassini/iss.py | 46 +++++---------------------------------- 1 file changed, 5 insertions(+), 41 deletions(-) diff --git a/oops/hosts/cassini/iss.py b/oops/hosts/cassini/iss.py index 1f765712..44a3d306 100755 --- a/oops/hosts/cassini/iss.py +++ b/oops/hosts/cassini/iss.py @@ -207,7 +207,7 @@ def from_index(filespec, **parameters): return snapshots #=============================================================================== -def initialize(ck='reconstructed', planets=None, offset_wac=False, asof=None, +def initialize(ck='reconstructed', planets=None, asof=None, spk='reconstructed', gapfill=True, mst_pck=True, irregulars=True): """Initialize key information about the ISS instrument. @@ -221,8 +221,6 @@ def initialize(ck='reconstructed', planets=None, offset_wac=False, asof=None, 'none' if the kernels are to be managed manually. planets A list of planets to pass to define_solar_system. None or 0 means all. - offset_wac True to offset the WAC frame relative to the NAC frame as - determined by star positions. asof Only use SPICE kernels that existed before this date; None to ignore. gapfill True to include gapfill CKs. False otherwise. @@ -231,7 +229,7 @@ def initialize(ck='reconstructed', planets=None, offset_wac=False, asof=None, irregulars True to include the irregular satellites; False otherwise. """ - ISS.initialize(ck=ck, planets=planets, offset_wac=offset_wac, asof=asof, + ISS.initialize(ck=ck, planets=planets, asof=asof, spk=spk, gapfill=gapfill, mst_pck=mst_pck, irregulars=irregulars) @@ -243,7 +241,6 @@ class ISS(object): fovs = {} initialized = False frames_defined = False - offset_wac = False # Exposed on the class so the generic Observation.set_cmatrix() and # get_cmatrix() can read the host's convention rotation via the @@ -360,7 +357,7 @@ class ISS(object): #=========================================================================== @staticmethod - def initialize(ck='reconstructed', planets=None, offset_wac=False, asof=None, + def initialize(ck='reconstructed', planets=None, asof=None, spk='reconstructed', gapfill=True, mst_pck=True, irregulars=True): """Initialize key information about the ISS instrument. @@ -375,8 +372,6 @@ def initialize(ck='reconstructed', planets=None, offset_wac=False, asof=None, managed manually. planets A list of planets to pass to define_solar_system. None or 0 means all. - offset_wac True to offset the WAC frame relative to the NAC frame - as determined by star positions. asof Only use SPICE kernels that existed before this date; None to ignore. gapfill True to include gapfill CKs. False otherwise. @@ -440,11 +435,6 @@ def initialize(ck='reconstructed', planets=None, offset_wac=False, asof=None, ISS.fovs[detector, 'SUM2'] = oops.fov.SubsampledFOV(full_fov_none, 2) ISS.fovs[detector, 'SUM4'] = oops.fov.SubsampledFOV(full_fov_none, 4) - # Remember the WAC offset option. The SPICE-derived camera frames are - # built lazily by define_camera_frames(), so an observation that supplies - # a custom C-matrix (without mapping) never depends on SPICE pointing. - ISS.offset_wac = offset_wac - ISS.initialized = True #=========================================================================== @@ -469,22 +459,6 @@ def define_camera_frames(): wac_frame_spice = oops.frame.SpiceFrame('CASSINI_ISS_WAC', frame_id='CASSINI_ISS_WAC_SPICE') - if ISS.offset_wac: - - # Apply offset for WAC relative to NAC - info = ISS.instrument_kernel['INS']['CASSINI_ISS_NAC'] - xfov = info['FOV_REF_ANGLE'] - yfov = info['FOV_CROSS_ANGLE'] - lines = info['PIXEL_LINES'] - samples = info['PIXEL_SAMPLES'] - - xpixel = np.arctan(np.tan(xfov * oops.RPD) / (samples/2.)) - ypixel = np.arctan(np.tan(yfov * oops.RPD) / (lines/2.)) - - # This is Rob's determination of WAC - NAC in units of NAC pixels - xshift = -7. * xpixel - yshift = 4.4 * ypixel - # Observation-facing camera frames: the raw *_SPICE pointing rotated # 180 degrees about the boresight (CMATRIX_ROTATION) into the oops-frame # convention. The *_SPICE SpiceFrames above stay registered as the @@ -497,17 +471,8 @@ def define_camera_frames(): # directly as object references below, not looked up by ID. nac_frame = oops.frame.Cmatrix(CMATRIX_ROTATION, nac_frame_spice, frame_id='CASSINI_ISS_NAC', override=True) - - if ISS.offset_wac: - wac_frame_spice_raw = oops.frame.Cmatrix(CMATRIX_ROTATION, wac_frame_spice, - frame_id='CASSINI_ISS_WAC-NO_OFFSET', - override=True) - wac_frame = oops.frame.Navigation((xshift,yshift), wac_frame_spice_raw, - frame_id='CASSINI_ISS_WAC', - override=True) - else: - wac_frame = oops.frame.Cmatrix(CMATRIX_ROTATION, wac_frame_spice, - frame_id='CASSINI_ISS_WAC', override=True) + wac_frame = oops.frame.Cmatrix(CMATRIX_ROTATION, wac_frame_spice, + frame_id='CASSINI_ISS_WAC', override=True) ISS.frames_defined = True @@ -523,7 +488,6 @@ def reset(): ISS.fovs = {} ISS.initialized = False ISS.frames_defined = False - ISS.offset_wac = False Cassini.reset() From 181298bde8f37efe81961686611467a9a2685e4d Mon Sep 17 00:00:00 2001 From: Joseph Spitale Date: Mon, 17 Aug 2026 10:50:08 -0700 Subject: [PATCH 20/22] Validate custom C-matrices and protect the frame registry Address findings F1, F2, F5 and F6 of the rms-oops #201 verification review: - F1: set_cmatrix() now rejects anything that is not a proper floating-point rotation -- non-finite values, boolean/integer data, reflections, scalings and non-orthogonal matrices all raise ValueError at the boundary instead of corrupting geometry downstream. - F2: set_cmatrix(frame_id=) only overrides a registration it made itself; a frame_id held by any other frame (e.g. CASSINI_ISS_NAC or J2000) raises instead of silently replacing its primary definition. Re-using a set_cmatrix-issued ID still re-points every observation sharing that frame, as documented. - F5: document that geometry derived before a set_cmatrix() call is not invalidated and must be rebuilt. - F6: document that a custom-cmatrix load furnishes no CK, so pxform against the SPICE camera frame at that epoch will fail. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_015Wg9zMfz6FNEotebnS1hmm --- oops/hosts/cassini/iss.py | 15 +++++- oops/observation/observation_.py | 71 +++++++++++++++++++++++++---- tests/hosts/cassini/iss/test_iss.py | 50 ++++++++++++++++++++ 3 files changed, 126 insertions(+), 10 deletions(-) diff --git a/oops/hosts/cassini/iss.py b/oops/hosts/cassini/iss.py index 44a3d306..e2643534 100755 --- a/oops/hosts/cassini/iss.py +++ b/oops/hosts/cassini/iss.py @@ -53,13 +53,24 @@ def from_file(filespec, fast_distortion=True, cspyce.pxform() or recorded in a CK (z along the line of sight, x left, y up). The fixed 180-degree rotation relating it to the oops-frame observation frame is - applied internally. None uses SPICE pointing as before. + applied internally. It must be a proper rotation with + floating-point values; anything else raises + ValueError. None uses SPICE pointing as before. + Note that with a custom cmatrix no CK is loaded, so + the SPICE camera frame has no pointing for this + epoch: the observation's pointing lives only in its + oops frame, and cspyce.pxform() against + CASSINI_ISS_ at this epoch will fail. frame_id when cmatrix is given, register the C-matrix under this frame ID and use only that (shared, registered) frame. None (default) gives the observation its own unregistered frame, so loading other images never - disturbs this one's pointing. Requires cmatrix. + disturbs this one's pointing. Requires cmatrix. An ID + already registered by anything other than a previous + custom-cmatrix load (e.g. 'CASSINI_ISS_NAC' or + 'J2000') raises ValueError rather than silently + replacing that frame. """ if cmatrix is None and frame_id is not None: diff --git a/oops/observation/observation_.py b/oops/observation/observation_.py index f79fca78..5370809b 100755 --- a/oops/observation/observation_.py +++ b/oops/observation/observation_.py @@ -491,6 +491,16 @@ def get_cmatrix(self, uv=None, time=None): return rotation.transpose() * oops_attitude #=========================================================================== + # Tolerance for accepting a supplied C-matrix as a proper rotation. + # Float64 rotation matrices (e.g. from cspyce.pxform) are orthonormal to + # ~1e-15, so this never rejects a legitimate matrix. + CMATRIX_TOL = 1.e-8 + + # Frame IDs registered by set_cmatrix() itself. set_cmatrix only overrides + # a registered frame whose ID appears here; it refuses to replace a frame + # registered by anything else (e.g. 'CASSINI_ISS_NAC' or 'J2000'). + _cmatrix_frame_ids = set() + def set_cmatrix(self, cmatrix, frame_id=None): """Set this observation's frame from a spice-convention C-matrix. @@ -499,9 +509,17 @@ def set_cmatrix(self, cmatrix, frame_id=None): 'host' subfield's CMATRIX_ROTATION attribute; get_cmatrix() applies its transpose). The resulting frame replaces self.frame. + Note that geometry already derived against the previous frame (e.g. + Backplanes) is not invalidated by this call; anything built before the + call still answers the old pointing and must be rebuilt. + Input: cmatrix an oops.Matrix3 (or 3x3 array) giving the spice-convention - C-matrix, as returned by get_cmatrix(). + C-matrix, as returned by get_cmatrix(). It must be a + proper rotation with floating-point values: non-finite + values, integer or boolean data (a signature of a + corrupted metadata record), reflections, and + non-orthogonal matrices all raise ValueError. frame_id None (default) attaches a fresh unregistered frame owned by the observation, so loading other images never disturbs its pointing; otherwise the frame is registered @@ -509,7 +527,9 @@ def set_cmatrix(self, cmatrix, frame_id=None): that re-using a frame_id re-points every observation that shares that registered frame; that sharing is the point of giving it an ID, but it is worth stating - explicitly. + explicitly. A frame_id already registered by anything + other than a previous set_cmatrix() call raises + ValueError rather than silently replacing that frame. """ try: @@ -519,15 +539,50 @@ def set_cmatrix(self, cmatrix, frame_id=None): 'class defines CMATRIX_ROTATION (the spice-frame ' '-> oops-frame convention rotation)') + # Validate the supplied C-matrix as a proper rotation. A recorded + # C-matrix arriving from a metadata file can carry NaNs, booleans, + # integer arrays, or a damaged matrix; each is an attributable error + # here rather than NaN or skewed geometry an hour downstream. + raw = cmatrix.values if isinstance(cmatrix, Qube) else np.asarray(cmatrix) + if not np.issubdtype(raw.dtype, np.floating): + raise ValueError('cmatrix must have floating-point values; got ' + 'dtype %s' % raw.dtype) + + matrix = Matrix3.as_matrix3(cmatrix) + values = matrix.values + if not np.all(np.isfinite(values)): + raise ValueError('cmatrix contains non-finite values') + + det = np.linalg.det(values) + ortho_error = np.max(np.abs(values.dot(values.T) - np.eye(3))) + if (abs(det - 1.) > Observation.CMATRIX_TOL + or ortho_error > Observation.CMATRIX_TOL): + raise ValueError('cmatrix is not a proper rotation matrix: ' + 'det = %.6g (should be 1), ' + 'max|C CT - I| = %.6g' % (det, ortho_error)) + # Convert the spice-convention C-matrix into the oops observation-frame # attitude by applying the fixed instrument convention rotation. - attitude = rotation * Matrix3.as_matrix3(cmatrix) + attitude = rotation * matrix - # frame_id=None attaches a fresh unregistered frame owned by this - # observation, isolated from the global registry. A given frame_id - # registers (or re-registers, with override=True) a single shared - # frame under that ID, cleanly replacing any prior primary definition. - frame = Cmatrix(attitude, frame_id=frame_id, override=(frame_id is not None)) + if frame_id is None: + # A fresh unregistered frame owned by this observation, isolated + # from the global registry. + frame = Cmatrix(attitude) + else: + # A given frame_id registers (or re-registers, with override=True) + # a single shared frame under that ID -- but only an ID that + # set_cmatrix itself handed out may be replaced. (A registry that + # was cleared via Frame.reset_registry() forgets prior ownership; + # _cmatrix_frame_ids may then permit an override it should not, + # which matches the debugging-only status of reset_registry.) + registered = frame_id in Frame.WAYFRAME_REGISTRY + if registered and frame_id not in Observation._cmatrix_frame_ids: + raise ValueError('frame_id %r is already registered by ' + 'another frame; choose a different ID' + % frame_id) + frame = Cmatrix(attitude, frame_id=frame_id, override=registered) + Observation._cmatrix_frame_ids.add(frame_id) self.frame = Frame.as_wayframe(frame) diff --git a/tests/hosts/cassini/iss/test_iss.py b/tests/hosts/cassini/iss/test_iss.py index a7c9e1b2..a496ae50 100644 --- a/tests/hosts/cassini/iss/test_iss.py +++ b/tests/hosts/cassini/iss/test_iss.py @@ -173,6 +173,56 @@ def test_reused_frame_id_replaces_primary(self): self.assertTrue(np.allclose(recovered.vals, custom2.vals)) self.assertFalse(np.allclose(recovered.vals, custom1.vals)) + #=========================================================================== + def test_colliding_frame_id_raises(self): + """A frame_id already registered by anything other than set_cmatrix + (e.g. the global camera frames or J2000) must raise rather than + silently replacing that frame's primary definition.""" + + baseline = from_file(self.filespec) + m0 = baseline.get_cmatrix() + camera = baseline.dict['INSTRUMENT_ID'][3:] + 'C' + spice0 = self._spice_cmatrix(camera, baseline.tstart) + custom = _PERTURBATION * spice0 + + with self.assertRaises(ValueError): + from_file(self.filespec, cmatrix=custom, + frame_id='CASSINI_ISS_' + camera) + + with self.assertRaises(ValueError): + from_file(self.filespec, cmatrix=custom, frame_id='J2000') + + # The refused registrations left the global pointing untouched. + plain = from_file(self.filespec) + self.assertTrue(np.allclose(plain.get_cmatrix().vals, m0.vals)) + + #=========================================================================== + def test_rejects_non_rotation_cmatrix(self): + """set_cmatrix (and therefore from_file's cmatrix argument) must + reject anything that is not a proper floating-point rotation: NaNs, + zeros, scalings, reflections, and boolean/integer data.""" + + obs = from_file(self.filespec, cmatrix=oops.Matrix3(np.eye(3))) + + bad_matrices = [ + np.diag([1., 2., 1.]), # scaling, det = 2 + np.diag([1., 1., -1.]), # reflection, det = -1 + np.full((3, 3), np.nan), # non-finite + np.zeros((3, 3)), # det = 0 + np.eye(3, dtype=bool), # boolean data + np.eye(3, dtype=int), # integer data + ] + for bad in bad_matrices: + with self.assertRaises(ValueError, msg=repr(bad)): + obs.set_cmatrix(bad) + + with self.assertRaises(ValueError): + from_file(self.filespec, cmatrix=np.diag([1., 1., -1.])) + + # A legitimate rotation still passes. + obs.set_cmatrix(_PERTURBATION) + self.assertTrue(np.allclose(obs.get_cmatrix().vals, _PERTURBATION.vals)) + #=========================================================================== def test_frame_id_without_cmatrix_raises(self): """frame_id is only meaningful together with cmatrix; passing it alone From 921628668d5ef2124942f0d184920c738b0390e9 Mon Sep 17 00:00:00 2001 From: Joseph Spitale Date: Mon, 17 Aug 2026 19:02:41 -0700 Subject: [PATCH 21/22] Extract C-matrix validation into Observation.validate_cmatrix Move the proper-rotation checks out of set_cmatrix() into a standalone static method that returns the validated Matrix3, so callers (e.g. a host's from_file) can validate a recorded C-matrix at any boundary without mutating an observation. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_015Wg9zMfz6FNEotebnS1hmm --- oops/observation/observation_.py | 67 ++++++++++++++++++++------------ 1 file changed, 42 insertions(+), 25 deletions(-) diff --git a/oops/observation/observation_.py b/oops/observation/observation_.py index 5370809b..5e12fb70 100755 --- a/oops/observation/observation_.py +++ b/oops/observation/observation_.py @@ -501,6 +501,45 @@ def get_cmatrix(self, uv=None, time=None): # registered by anything else (e.g. 'CASSINI_ISS_NAC' or 'J2000'). _cmatrix_frame_ids = set() + @staticmethod + def validate_cmatrix(cmatrix): + """Validate a C-matrix as a proper rotation; return it as a Matrix3. + + A recorded C-matrix arriving from a metadata file can carry NaNs, + booleans, integer arrays, or a damaged matrix; each raises an + attributable ValueError here rather than producing NaN or skewed + geometry an hour downstream. + + Input: + cmatrix an oops.Matrix3 (or 3x3 array). It must be a proper + rotation with floating-point values: non-finite values, + integer or boolean data (a signature of a corrupted + metadata record), reflections, and non-orthogonal + matrices all raise ValueError. + + Return: the validated C-matrix as a Matrix3. + """ + + raw = cmatrix.values if isinstance(cmatrix, Qube) else np.asarray(cmatrix) + if not np.issubdtype(raw.dtype, np.floating): + raise ValueError('cmatrix must have floating-point values; got ' + 'dtype %s' % raw.dtype) + + matrix = Matrix3.as_matrix3(cmatrix) + values = matrix.values + if not np.all(np.isfinite(values)): + raise ValueError('cmatrix contains non-finite values') + + det = np.linalg.det(values) + ortho_error = np.max(np.abs(values.dot(values.T) - np.eye(3))) + if (abs(det - 1.) > Observation.CMATRIX_TOL + or ortho_error > Observation.CMATRIX_TOL): + raise ValueError('cmatrix is not a proper rotation matrix: ' + 'det = %.6g (should be 1), ' + 'max|C CT - I| = %.6g' % (det, ortho_error)) + + return matrix + def set_cmatrix(self, cmatrix, frame_id=None): """Set this observation's frame from a spice-convention C-matrix. @@ -516,10 +555,8 @@ def set_cmatrix(self, cmatrix, frame_id=None): Input: cmatrix an oops.Matrix3 (or 3x3 array) giving the spice-convention C-matrix, as returned by get_cmatrix(). It must be a - proper rotation with floating-point values: non-finite - values, integer or boolean data (a signature of a - corrupted metadata record), reflections, and - non-orthogonal matrices all raise ValueError. + proper rotation with floating-point values; anything + else raises ValueError (see validate_cmatrix()). frame_id None (default) attaches a fresh unregistered frame owned by the observation, so loading other images never disturbs its pointing; otherwise the frame is registered @@ -539,27 +576,7 @@ def set_cmatrix(self, cmatrix, frame_id=None): 'class defines CMATRIX_ROTATION (the spice-frame ' '-> oops-frame convention rotation)') - # Validate the supplied C-matrix as a proper rotation. A recorded - # C-matrix arriving from a metadata file can carry NaNs, booleans, - # integer arrays, or a damaged matrix; each is an attributable error - # here rather than NaN or skewed geometry an hour downstream. - raw = cmatrix.values if isinstance(cmatrix, Qube) else np.asarray(cmatrix) - if not np.issubdtype(raw.dtype, np.floating): - raise ValueError('cmatrix must have floating-point values; got ' - 'dtype %s' % raw.dtype) - - matrix = Matrix3.as_matrix3(cmatrix) - values = matrix.values - if not np.all(np.isfinite(values)): - raise ValueError('cmatrix contains non-finite values') - - det = np.linalg.det(values) - ortho_error = np.max(np.abs(values.dot(values.T) - np.eye(3))) - if (abs(det - 1.) > Observation.CMATRIX_TOL - or ortho_error > Observation.CMATRIX_TOL): - raise ValueError('cmatrix is not a proper rotation matrix: ' - 'det = %.6g (should be 1), ' - 'max|C CT - I| = %.6g' % (det, ortho_error)) + matrix = Observation.validate_cmatrix(cmatrix) # Convert the spice-convention C-matrix into the oops observation-frame # attitude by applying the fixed instrument convention rotation. From 7f501f68ff28eb12d373d255009dc1bd1006a235 Mon Sep 17 00:00:00 2001 From: Joseph Spitale Date: Tue, 18 Aug 2026 08:50:39 -0700 Subject: [PATCH 22/22] Pin the frame_id=None isolation contract with a registry test The per-observation ownership documented for set_cmatrix(frame_id=None) is emergent from how the current registry treats unregistered frames, not a property Cmatrix or Frame.register promises. PR #203's registry rewrite dedups equal-valued unregistered frames to a shared wayframe and retains every construction globally, which would falsify the contract without anything failing. Pin it: equal-valued C-matrices yield distinct frame objects, and a default load leaves the wayframe registry and frame cache unchanged. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_015Wg9zMfz6FNEotebnS1hmm --- tests/hosts/cassini/iss/test_iss.py | 36 +++++++++++++++++++++++++++++ 1 file changed, 36 insertions(+) diff --git a/tests/hosts/cassini/iss/test_iss.py b/tests/hosts/cassini/iss/test_iss.py index a496ae50..111f67b5 100644 --- a/tests/hosts/cassini/iss/test_iss.py +++ b/tests/hosts/cassini/iss/test_iss.py @@ -108,6 +108,42 @@ def test_default_frame_is_unregistered_and_isolated(self): self.assertFalse(np.allclose(obs1.get_cmatrix().vals, obs2.get_cmatrix().vals)) + #=========================================================================== + def test_default_frames_are_distinct_and_stay_out_of_registry(self): + """Pin the frame_id=None isolation contract against registry rewrites: + observations given equal-valued C-matrices must get distinct frame + objects, and a default set_cmatrix must leave the global frame + registry and cache unchanged. A registry that dedups or retains + unregistered frames (e.g. a per-subclass wayframe table) breaks the + documented per-observation ownership; this test makes that a failure + here instead of a silent semantic change.""" + + baseline = from_file(self.filespec) + camera = baseline.dict['INSTRUMENT_ID'][3:] + 'C' + spice0 = self._spice_cmatrix(camera, baseline.tstart) + custom = _PERTURBATION * spice0 + + # Warm every lazy path once (load + get_cmatrix) before measuring. + obs1 = from_file(self.filespec, cmatrix=custom) + _ = obs1.get_cmatrix() + + n_wayframes = len(Frame.WAYFRAME_REGISTRY) + n_cached = len(Frame.FRAME_CACHE) + + # Equal-valued C-matrices: distinct frame objects per observation. + obs2 = from_file(self.filespec, cmatrix=custom) + obs3 = from_file(self.filespec, cmatrix=custom) + _ = obs2.get_cmatrix() + _ = obs3.get_cmatrix() + + self.assertIsNot(obs1.frame, obs2.frame) + self.assertIsNot(obs2.frame, obs3.frame) + self.assertTrue(np.all(obs2.get_cmatrix().vals == custom.vals)) + + # No global retention: the registry and cache did not grow. + self.assertEqual(len(Frame.WAYFRAME_REGISTRY), n_wayframes) + self.assertEqual(len(Frame.FRAME_CACHE), n_cached) + #=========================================================================== def test_default_does_not_leak_into_plain_load(self): """A default custom load never touches the global camera frame, so a