Add Fittable/mutable framework, rewrite the Path and Frame registries, require Python 3.11

[markshowalter]

Purpose

This branch introduces a Fittable/mutable framework for in-place parameter fitting, rewrites the Path and Frame registries around it, and fixes the regressions that rewrite introduced. It also drops Python 3.8 through 3.10.

The Fittable work is the motivation: pointing corrections, time shifts, plate scales and orbital elements need to be adjustable in place, with any object that depends on them refreshing itself when they change. Supporting that required reworking how Paths and Frames are registered, cached and linked.

A second pass repairs what the rewrite left behind in the QuickPath and QuickFrame machinery, in LinkedPath, PathShift, Navigation and KeplerPath, and settles the API of the four linkable Fittable classes.

A third pass puts the framework to work: an observation can now be pointed by a C matrix of its own rather than by a C kernel, which is what image navigation needs, and can be copied so that a fit applied to the copy leaves the original alone. Getting that right exposed further defects in the mutable framework and in OffsetFOV.

A fourth pass settles how the framework is reached. The mutable API had been a set of module-level functions called as mutable.refresh(obj); the seven OOPS base classes now inherit it as methods.

Changes / Implementation Details

Fittable and mutable framework

oops/fittable.py is rebuilt around set_params, refresh and freeze. The new oops/mutable.py propagates staleness through objects that merely contain a Fittable sub-object, tracking state through injected _FITTABLE*/_MUTABLE* attributes and an integer version counter. PathShift, FrameShift, TimeShift and Platescale are new; OffsetFOV, Navigation, Rotation and KeplerPath are converted. The four new classes are marked as untested placeholders.

Path and Frame registry rewrite

Wayframe, Waypoint, AliasPath, AliasFrame and RelativeFrame are gone, replaced by NullFrame/J2000Frame, NullPath/SSBPath, ReversedFrame, RelativePath, ReversedPath and RotatedPath. Registries and the registration hook are private. QuickFrame and QuickPath move into their own modules. A new oops/cache.py provides an LRU cache whose clean_key() makes polymath objects usable as dictionary keys.

Fixes to that rewrite

• mutable._refresh_internal never applied _refresh() on an object's first pass, so the mutable.refresh(self) at the end of every constructor did nothing and derived attributes such as _transform, _times and _pos_x were never created.
• Five Path utility classes cached themselves in Frame._FRAME_CACHE rather than Path._PATH_CACHE, leaving the path cache unused and the frame cache holding Paths. Path._wrt lacked the reversal branch its Frame counterpart has, so linking a root path to one of its descendants recursed until the stack overflowed.
• RelativePath discarded the origin Path it needs for the subtraction, and advertised the frame of the wrong Path.
• LinkedFrame's origin check rejected a null frame origin, which is the ordinary case.
• The "already quick" guards were restored to QuickPath.for_path and QuickFrame.for_frame, and extend() repaired in both -- a dict read as a method, frame keys used in a path class, a stale _steps, arrays whose lengths disagreed, a seam that left the tabulated times non-monotonic, and a tuple passed to a two-argument signature.
• SpicePath.get() ignored origin and frame when asked for the SSB. SPICE accepts several names per body, so the name a caller uses is now registered alongside the canonical one; "GLL" resolves to GALILEO_ORBITER.
• KeplerPath had eleven attribute names left behind by an incomplete privatization.
• os.path.basename() was applied to an FCPath in the Juno hosts, which is not os.PathLike; these now use the FCPath.name property as the other hosts do.

QuickPath and QuickFrame reach the objects that need them

QuickFrames were reaching only SpiceFrame, so a RingFrame built on one fell back to a SPICE call per evaluation, and the photon solvers then evaluated it with quick=False, which also suppressed the QuickFrame SpiceFrame builds for itself. _USE_QUICKFRAMES now describes a Frame's own time dependence -- a Frame returning a fixed Transform has nothing to tabulate -- and LinkedFrame/ReversedFrame inherit it from the frames they combine. RingFrame, PoleFrame, TrackerFrame, LaplaceFrame, InclinedFrame and SynchronousFrame opt in. The same treatment is applied to Paths: the four composite Path classes propagate _USE_QUICKPATHS, and KeplerPath opts in.

Three latent defects surfaced along the way:

• LinkedFrame.transform_at_time_if_possible referenced self.parent and self.frame, which no longer exist, and then dropped the quick argument it was passed. Both were unreachable while LinkedFrame was never tabulated.
• QuickFrame sampled its slow frame without quick=False, so tabulating a SpiceFrame made that frame quicken itself for the tabulation times -- one QuickFrame built two, at twice the SPICE calls. QuickPath already passed quick=False in the same places.
• QuickFrame splined the tabulated quaternions without making them continuous in sign. Since q and -q describe the same rotation, any frame turning through more than pi within the tabulated window returned noise. IAU_JUPITER would have broken past a window of roughly five hours.

Both classes now report a meaningful error when asked to tabulate an object whose state does not vary with time, rather than an IndexError or a ValueError from scipy.

Path and Frame repairs

• LinkedPath took its frame from the path being linked rather than the parent it is linked to, though event_at_time rotates into the parent's frame before adding. Invisible while both were J2000; through SpicePath._get_shortcut it produced a path claiming J2000 while returning ring-frame events, and any Event built from it failed with "Events must share a common frame for path addition".
• PathShift and Navigation raised AttributeError from their constructors and could not be built at all: self.link for self._link, _FRAME_IDS where the registration machinery looks for _WAYFRAMES, a test of an _epoch attribute that is never defined, and _angles left unset when constructed as a link.
• KeplerPath set its origin to the observer but event_at_time returned a position measured from the planet, so a photon solved through the base solver reported a light travel time of 0.54 s where the correct value is 4400.8 s. photon_to_event built its ray from Event.pos_j2000, which does not exist, assigned the light travel times with the signs reversed, and applied the light time a second time by passing an already-retarded time back into event_at_time.

The placeholder classes

FrameShift and Platescale were labelled PLACEHOLDER CODE ... NOT YET TESTED and neither worked. FrameShift.transform_at_time read self.frame, an attribute Frame does not have, so it could not produce a transform at all, and its "+" frame_id branch asked a Frame for _path_id. Platescale never declared nparams and named its parameter property _params, so set_params raised; it also copied uv_area from the reference FOV without scaling it, leaving area_factor() wrong by the square of the plate scale. Fittable.set_params itself read self._nparams when reporting a wrong parameter count, turning that ValueError into an AttributeError. All three classes with placeholder labels -- FrameShift, Platescale and PathShift -- now have test modules and the labels are gone.

Pointing an observation without a C kernel

cassini.iss.from_file() accepts a frame of its own and a navigation option that wraps it in a Navigation, so an observation can be pointed by a fitted C matrix. The SPICE camera frames are built lazily by ISS.define_camera_frames(), so an observation with a custom frame never constructs them, and Cassini.used_kernels() takes a ck option so that a CK which happens to be furnished is not reported as used by an observation whose pointing did not come from one. The rotation between the SPICE and oops frame conventions is recorded as iss.CMATRIX_ROTATION and attached to each observation as its spice_to_cmatrix subfield.

Observation gains get_spice_cmatrix() and set_spice_cmatrix(), which read and write the pointing in the convention of the SPICE toolkit's C kernel, and set_frame(), which replaces the frame in place and refuses to act on a frozen observation. An observation that is not mutable reports itself as frozen because it has nothing to freeze, so that guard tests both. Cmatrix and PosTargFrame gain the _WAYFRAMES registry and _wayframe_key() that let two identically defined frames share a wayframe.

Copying an observation, and copying a Fittable

Observation.copy() returns an observation that shares the data array, and shares the frame, path, FOV and cadence because those are canonical registered objects, but has its own subfield dictionary and none of the original's mutable-state record. A Fittable sub-object is duplicated, so fitting one observation leaves the other alone. navigate() is rebuilt on it. It previously took a shallow copy of __dict__, which left the two observations sharing one subfield dictionary and one mutable-state record; because the record is keyed by attribute name while the two frames are distinct objects, it could report that an observation was up to date after its frame had genuinely changed.

Duplicating a Fittable had no general mechanism, so Fittable.copy() is new. copy.copy() goes through __setstate__, which freezes the result and re-registers it under the original's ID -- right for unpickling, wrong for copying. copy() instead reconstructs from the constructor arguments __getstate__ already returns, dropping the trailing ID of a registered Frame or Path so the copy is left unregistered. KeplerPath overrides it, being the one class whose constructor cannot take its state positionally.

Further repairs to the mutable framework

_refresh() is free to replace a sub-object with a newly constructed one, but the replacement had never been refreshed itself, so it reported that it was stale on every later test, and so did the object holding it. A Backplane over a Navigation-framed observation therefore never settled: needs_refresh() stayed True and every refresh() recomputed the observation event, the gridless event and all cached backplanes.

Fixing that exposed a latent defect. _get_info() and _refresh_internal() both guard against cycles in the object graph; _needs_refresh_internal() did not, and was saved only by returning early before it walked deep enough to reach one. An Event whose _wod_ refers to itself sends it into unbounded recursion, so it now carries the same guard. _invalidate() is added to discard the cached record of which sub-objects are mutable, which is otherwise determined once and never revisited, so that replacing a sub-object in place is noticed.

OffsetFOV

The constructor accepts only one of uv_offset and xy_offset, deriving the other from it, but __getstate__ returned both and __setstate__ fed both back, so pickling an OffsetFOV raised ValueError. It now saves only uv_offset. The constructor also stored the offset exactly as given, so an offset supplied as a tuple left uv_offset without the Pair interface that params and set_params depend on; both offsets are now coerced with Pair.as_pair().

The mutable API becomes an interface

Mutable is a base class that publishes the mutable API as methods, and Backplane, Cadence, FOV, Frame, Observation, Path and Surface inherit it. Seventy-six calls to the module-level functions become method calls and twenty-six imports of oops.mutable fall away; the functions remain, and are what the methods delegate to.

The six query methods carry a leading underscore -- _is_frozen, _is_mutable, _needs_refresh, _mutable_names, _unfrozen_names and _version -- so that they do not shadow the is_frozen and version properties that Fittable publishes. A class such as Navigation inherits from both, with Mutable ahead of Fittable in the MRO, so an unprefixed is_frozen() resolves to the method; the bound method is always truthy, set_params() refuses every fit, and Navigation and FrameShift discard their links on construction. refresh() and freeze() are left unprefixed, both classes defining them and both delegating to the same functions, so the MRO's choice makes no difference.

API consistency

PathShift, TimeShift, FrameShift and Navigation all hold their linked object in _link and publish the same link property; TimeShift also moves its offset to _dt behind a dt property, matching the other two shift classes, and is now exported from oops.cadence. Every __init__.py declares its public API in __all__, and 21 dead imports are removed.

Python version

requires-python moves to >=3.11 and both CI matrices now begin at 3.11, testing 3.11 through 3.13 on Linux, macOS and Windows.

Type of Change

• Bug fix
• New feature
• Breaking change
• Refactor
• Documentation
• Tests
• CI/build

Testing

All three entry points pass:

python -m unittest tests/unittester.py               62 tests   OK
python -m unittest tests/hosts/unittester.py          5 tests   OK
python -m unittest tests/unittester_with_hosts.py    67 tests   OK

Eight test modules are new -- test_quickframe, test_quickpath, test_pathshift, test_navigation, test_timeshift, test_frameshift, test_platescale and test_offsetfov -- covering classes that had no coverage at all, five of which could not previously be constructed or used. Each new assertion was checked against the unfixed source to confirm it fails there. For test_offsetfov that was done once per defect, since the coercion failure aborts the test before it reaches the pickle assertions and would otherwise mask them.

The main suite went from 19 failing tests to zero, and the host gold-master suite from four of five failing to zero. The Cassini ISS and Galileo SSI gold masters both compare clean, which exercises observation loading, backplane generation and comparison end to end.

Beyond the suite, the reversal path in Path._wrt was checked against independently computed geometry: earth.wrt(moon) matches both -moon.wrt(earth) and a separately constructed SpicePath('EARTH','MOON') to zero residual across 121 epochs. QuickPath.extend and QuickFrame.extend have no test coverage at all, so they were driven directly and their interpolation checked against the underlying slow path and frame, agreeing to 4e-16 relative and 1e-15 absolute respectively.

The pointing work has no gold master of its own, so it was checked against SPICE directly. Across four Cassini ISS images, two NAC and two WAC, with and without a Navigation frame, get_spice_cmatrix() reproduces cspyce.pxform('J2000', 'CASSINI_ISS_<camera>', midtime) to zero residual. set_spice_cmatrix() inverts it to zero residual from a Matrix3, from a numpy array and from a list of lists, and an observation pointed that way yields right ascension and declination identical to the CK-pointed observation. Fittable.copy() was exercised over all eight concrete Fittable classes, confirming that each copy starts with the original's parameters, comes back unfrozen and unregistered, and can be fitted without disturbing the original.

The Mutable conversion is mostly mechanical, but its one real hazard -- a name on Mutable shadowing a same-named Fittable property through the MRO -- is barely covered by the suite; the is_frozen collision surfaced in only three tests, all of them Frame tests, and would have gone unnoticed on the FOV and Cadence conversions. So all 82 Mutable subclasses were checked directly: for each of the eight names, every provider along the MRO was compared by descriptor kind, and no class has two providers of differing kind. Every Fittable subclass now inherits Mutable, and all eight concrete ones were exercised through both interfaces at once, confirming that the is_frozen property and _is_frozen() agree and that copy() still returns the right class.

Several tests were themselves unsound and were fixed: the SpicePath tests restored Path._USE_QUICKPATHS to True in tearDown when the class default is False, corrupting later tests through the base class, and test_spice_shape never cleared the Path registry, so its custom path ID was ignored whenever an earlier test had already registered VENUS.

Potential Impacts

Breaking, public API. Wayframe, Waypoint, AliasPath, AliasFrame and RelativeFrame no longer exist. Path.as_path(id) now returns the registered Path rather than a zero-position Waypoint; use NullPath for the old behaviour. AliasPath(path, frame) becomes NullPath(path, frame=frame). The registries are private (_PATH_REGISTRY, _FRAME_REGISTRY, _register(), _reset_caches()), the cross-class attributes injected by oops/__init__.py are renamed from XXX_CLASS to _Xxx, quick is keyword-only and defaults to None, the QUICK dictionary keys quickpath_cache/quickframe_cache gain a _size suffix, automatic TEMPORARY_* path IDs are gone (path_id is None when unregistered; use is_registered), and SpicePath.USE_SPICEPATH_SHORTCUTS no longer exists, so there is no longer a way to disable SPICE shortcuts for debugging.

Public API, new base class. Backplane, Cadence, FOV, Frame, Observation, Path and Surface all gain Mutable as a base class, so every Frame, Path, Surface, FOV, Cadence, Observation and Backplane in the package acquires eight public or semi-public methods it did not have. Nothing is removed and the module-level functions still work, so this is additive; but a subclass outside this repository that defines its own refresh, freeze or _version will now override the inherited version rather than sit alongside it, and one defining an is_frozen method will hit the shadowing described above.

Breaking, Cassini ISS. cassini.iss.from_file() takes its options as keyword arguments only, and the offset_wac option is gone from both iss.initialize() and ISS.initialize(). The CASSINI_ISS_NAC and CASSINI_ISS_WAC frames are no longer built as a side effect of initialize(); ISS.define_camera_frames() builds them, and from_file() and from_index() call it for themselves.

Additions. Observation gains copy(), set_frame(), get_spice_cmatrix() and set_spice_cmatrix(). Observation.navigate() keeps its signature but now returns a properly independent copy. Fittable gains copy(), inherited by all nine subclasses. OffsetFOV.__getstate__ returns a two-element state rather than three, so a pickle written by an older version will not load -- though no such pickle can exist, because writing one raised.

Python support. 3.8, 3.9 and 3.10 are dropped. oops/cache.py uses a match statement, so the floor cannot go back below 3.10 without rework.

Performance. Path._PATH_CACHE was never being written to and so never hit; paths were rebuilt on every call. It now works as intended.

The Quick* changes were measured on a Cassini ISS ring-plane solve over 1,000,000 rays, with the observer in a CK-driven frame. The default path took 12.03 s and 4,000,412 SPICE calls; it now takes 1.05 s and 822. Solving for a photon from a KeplerPath over 300,000 rays went from 64.92 s to 0.65 s, with identical light times. Tabulating a frame or path costs 219 SPICE calls rather than 427, the nested QuickFrame having been removed.

Two measurements worth recording, since they contradict the intuition that eliminating SPICE calls is what matters. A QuickFrame is worth 2.2x on a CK-driven frame but nothing at all on a text PCK, where interpolating costs as much as the SPICE call. A QuickPath is worth about 100x on the time spans a real observation spans, because quickpath_linear_interpolation_threshold collapses it to linear interpolation, but it loses to raw SPICE for spans beyond about 25 s when the evaluation times are unsorted -- scipy's spline evaluation walks the knots. Neither case arises in normal use, but both are easy to hit in a synthetic benchmark.

Checklist

• Code follows project style (ruff check, ruff format) — n/a: this repository lints with flake8, not ruff; there is no ruff.toml and no [tool.ruff] in pyproject.toml. Repository-wide, flake8 oops falls from 283 findings on main to 104 here and flake8 spicedb is unchanged at 47; F401 goes from 135 to 0. The change introduces no new findings.
• Type annotations present and mypy passes — n/a: no mypy configuration exists in the repository, and annotations are confined to fittable.py and mutable.py by design.
• No secrets or credentials committed
• No warnings or errors introduced (CI, linters, type checking, builds) or justified in Notes — see the flake8 counts above.
• Docstrings and Sphinx docs updated (if applicable) — docstrings were updated where behaviour changed, including the Raises: clause of RotatedPath. There is no Sphinx docs tree in this repository.
• No temporary or debug code left in — two stray print statements dumping __dict__ were removed from tests/path/test_spicepath.py.
• Performance impact assessed (see Potential Impacts above)
• Breaking changes flagged in Type of Change above

Notes

Follow-up work, none of it blocking:

• KeplerPath solves its light travel time to the planet centre rather than to the body, so the ray length and the light travel time agree only to the radial part of the orbital offset over the range, about 1e-05 for the orbit used in the tests. That is inherent to the class, not a defect.
• JunoCam and JIRAM remain out of the test suite by choice. tests/hosts/juno/jiram/__init__.py has been migrated to the current gold-master API and the FCPath crash fixed, so it can be enabled when wanted, but its April 2023 gold masters disagree with this branch on sky angles and ring geometry and would need triage first.
• The Juno gold-master directories need renaming in the resource tree; see Gold-master directory rename below. This is the one change in this PR that git cannot carry.
• Matrix3 * ndarray returns a Scalar of shape (3, 3) in polymath rather than performing matrix multiplication, and does so silently. Observation.set_spice_cmatrix() guards its own call site with Matrix3.as_matrix3(), but the trap remains anywhere else a Matrix3 meets a raw array, and is worth an issue against rms-polymath.
• Observation.set_spice_cmatrix() replaces the frame outright, so calling it on an observation created with navigation=True discards the fittable frame rather than re-basing it. That suits setting an absolute pointing, but it silently resets a fit in progress.
• Observation.copy() shares a non-Fittable frame, path, FOV and cadence deliberately, but shares a nested Fittable too -- a Cmatrix whose reference is a Navigation, say. Only a directly held Fittable is duplicated.
• run-lint.yml has its pull_request/push triggers commented out and pointing at a master branch that does not exist here, so flake8 has never run in CI.
• tests/hosts/unittester.py covers two of the seven instrument packages; juno, hst, voyager, newhorizons and keck are all commented out.

Gold-master directory rename

Gold masters are stored under a directory named after the module string, used verbatim as a path component (oops/gold_master/__init__.py):

self.gold_dir = GOLD_MASTER_PREFIX / args.module / basename_prefix

The Juno masters were adopted in April 2023, before the host packages were renamed from hosts.* to oops.hosts.*; the Cassini and Galileo masters were re-adopted in December 2023 and already use the new name. So the Juno tests ask for oops.hosts.juno.* and find nothing, reporting No gold master for every backplane.

The resource tree is not a git repository, so this has to be applied by hand wherever the resources live, including the self-hosted CI runners:

cd "${OOPS_GOLD_MASTER_PATH:-$OOPS_RESOURCES/gold_master}"
mv hosts.juno.jiram   oops.hosts.juno.jiram
mv hosts.juno.junocam oops.hosts.juno.junocam

Afterwards all four directories share one convention:

oops.hosts.cassini.iss
oops.hosts.galileo.ssi
oops.hosts.juno.jiram
oops.hosts.juno.junocam

The rename is reversible, and it is a no-op for anyone who does not run the Juno tests, since JunoCam and JIRAM are deliberately excluded from tests/hosts/unittester.py. Note that once the masters are found, the JIRAM comparisons do run but disagree with this branch on sky angles and ring geometry, as noted above.

🤖 Generated with Claude Code

[coderabbitai]

Important

• 🔍 Trigger review

This repository does not receive automatic reviews because it has fewer than 10 stars.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 2565d205-e87f-45c3-8db0-578674e0e435

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[codecov]

Codecov Report

❌ Patch coverage is 70.90569% with 1089 lines in your changes missing coverage. Please review.
✅ Project coverage is 78.90%. Comparing base (ebce0e5) to head (f9e939b).
⚠️ Report is 23 commits behind head on main.

Files with missing lines	Patch %	Lines
oops/frame/quickframe.py	66.76%	90 Missing and 18 partials ⚠️
oops/frame/spicetype1frame.py	30.76%	86 Missing and 4 partials ⚠️
oops/path/quickpath.py	61.94%	72 Missing and 14 partials ⚠️
oops/frame/frame_.py	72.20%	64 Missing and 13 partials ⚠️
oops/mutable.py	80.69%	33 Missing and 17 partials ⚠️
oops/frame/spiceframe.py	77.66%	36 Missing and 8 partials ⚠️
oops/frame/laplaceframe.py	21.81%	43 Missing ⚠️
oops/frame/rotation.py	48.10%	35 Missing and 6 partials ⚠️
oops/path/keplerpath.py	83.82%	33 Missing and 5 partials ⚠️
oops/observation/observation_.py	31.91%	32 Missing ⚠️
... and 59 more

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #203      +/-   ##
==========================================
+ Coverage   75.40%   78.90%   +3.50%     
==========================================
  Files         192      211      +19     
  Lines       24094    25868    +1774     
  Branches     2926     2812     -114     
==========================================
+ Hits        18168    20412    +2244     
+ Misses       5088     4593     -495     
- Partials      838      863      +25     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[rfrenchseti]

Exercised this branch as a drop-in replacement for main (via PYTHONPATH, everything else held fixed). Three things worth flagging, one of them a hard break.

1. oops.hosts.newhorizons.lorri cannot be imported

$ python -c "import oops.hosts.newhorizons.lorri"
  File ".../oops/hosts/newhorizons/lorri.py", line 495, in initialize
    ignore = oops.frame.Cmatrix(flipxyz, lorri_flipped, 'NH_LORRI')
TypeError: Cmatrix.__init__() takes from 2 to 3 positional arguments but 4 were given

Cmatrix.__init__ is (self, cmatrix, reference=None, *, frame_id=None) on this branch, so frame_id is keyword-only, but lorri.py:495 still passes it positionally. Since LORRI.initialize() runs at module scope (line 521), the module is unimportable rather than merely broken at call time — any downstream import of the LORRI host fails outright.

This looks like the one straggler from the keyword-only migration: cassini/iss.py, voyager/iss.py, and juno/jiram/__init__.py all pass frame_id= correctly. It isn't caught by CI because the NewHorizons host tests are commented out in tests/hosts/unittester.py:12. The fix is frame_id='NH_LORRI'.

2. Unregistered frames are retained for the life of the process

Frame._register() inserts into the per-subclass _WAYFRAMES dict unconditionally, and constructing the Transform populates Frame._FRAME_CACHE — both happen whether or not a frame_id was supplied. Both containers are plain dicts, so a process that builds many distinct frames grows without bound:

import numpy as np
from polymath import Matrix3
from oops.frame import Frame
from oops.frame.cmatrix import Cmatrix

rng = np.random.default_rng(0)
for _ in range(10_000):
    Cmatrix(Matrix3(np.linalg.qr(rng.normal(size=(3, 3)))[0]))   # note: no frame_id

print(len(Cmatrix._WAYFRAMES), len(Frame._FRAME_CACHE))

On this branch that prints 10000 20002, and maxrss goes from 142 MB to 165 MB — about 2.3 KB retained per frame. On main (36c1502) the identical loop leaves FRAME_CACHE and WAYFRAME_REGISTRY at 2 entries each; only the temporary-id counter moves.

So it is a slow leak rather than a dramatic one, but it is unbounded and scales with the number of distinct frames a process constructs, which makes it a problem for any long-running or batch-style use. Frame._reset_caches() does clear both dicts, but it also drops every registered frame, so it isn't usable as a periodic trim mid-run.

The branch already seems to carry the intended fix: oops/cache.py defines a bounded Cache (maxsize, LRU eviction once ~10% over), and _register() already routes these keys through Cache.clean_key(). It looks like only the containers themselves were left as plain dicts.

3. Two behavior/API changes that deserve a line in the PR description

Equal-valued unregistered frames now dedup to one object. Cmatrix(m)._wayframe is Cmatrix(m)._wayframe is True here and False on main. Any caller that treats an unregistered frame as private to whatever constructed it now shares one with every other caller holding an equal value. Immutability makes that mostly harmless, but it is observable, and it silently weakens "unregistered" from "isolated" to "not addressable by ID".

Public class attributes were privatized with no aliases: Frame.FRAME_CACHE -> Frame._FRAME_CACHE, Frame.WAYFRAME_REGISTRY -> Frame._FRAME_REGISTRY, and Frame.TEMPORARY_FRAME_ID is gone entirely (unregistered frames no longer consume an id). Downstream code that inspects any of these fails with AttributeError. Presumably intentional, but it is a breaking change and isn't currently called out.

[rfrenchseti]

See earlier comment

[rfrenchseti]

Follow-up to my earlier comment: there is also a significant performance regression on this branch, concentrated in frames that aren't SpiceFrame. Numbers below are this branch (284ed66) against main at 36c1502, same machine, BLAS/OMP pinned to one thread, nothing else running.

Symptom

On a geometry workload that evaluates ring surface intercepts across a full Cassini ISS frame, wall clock goes 25.1 s -> 53.5 s (2.1x). A second workload over the same image set that is dominated by FFT work outside oops is unchanged (33.69 s -> 33.81 s), and a mixed 75-image batch comes out at 1.30x overall — so this is not a flat per-call overhead, it is specific to certain frames.

Where it goes

cProfile on the ring case, main vs this branch:

	main	this branch
cspyce._cspyce0.sxform calls	37 463	4 404 257
time in sxform	0.24 s	23.17 s
ringframe.py:127 transform_at_time calls	2	78
total function calls	23.2M	58.8M

Instrumenting the size of the time array arriving at SpiceFrame.transform_at_time explains the count exactly: a 1 465 696-element array is evaluated 3 times on main and 6 times here, and on this branch roughly three of those evaluations fall through to the per-element loop rather than being served by an interpolating frame. 3 x 1 465 696 = 4.40M, which is the sxform count. On main the equivalent evaluations are served by a QuickFrame.

Proximate cause

Frame._USE_QUICKFRAMES (frame_.py:94) defaults to False, and QuickFrame.for_frame() returns the frame unsubstituted when it is not set:

if not frame._USE_QUICKFRAMES:
    return frame

Across the whole tree only two classes opt in — SpiceFrame._USE_QUICKFRAMES = True (spiceframe.py:24) and SpicePath._USE_QUICKPATHS = True (spicepath.py:23). Every other Frame and Path subclass therefore loses QuickFrame/QuickPath substitution entirely. Wrapping QuickFrame.for_frame and counting outcomes on the ring case confirms it: SpiceFrame is substituted 347 times, and RingFrame is substituted 0 of 57 times while being handed the 1 465 696-element array 5 times.

On main there is no such gate — Frame.quick_frame() is eligible to build a QuickFrame for any frame — which is why the same workload never reaches SPICE per element.

One caveat, since it is the obvious thing to try

Setting RingFrame._USE_QUICKFRAMES = True is not a fix. It does remove the cost (53.5 s -> 1.6 s), but it also changes the answer: a derived image-space quantity in my workload moved by roughly 0.8 px, which is far outside the ~1e-10 agreement the branch otherwise shows against main on the same image. Presumably the default interpolation grid is too coarse for a frame with the ring's rotation rate, so the flag can't simply be switched on per class without also settling the time step. Which frames should be quick-frameable, and on what grid, looks like a design decision for whoever did the rewrite rather than something to patch blindly.

Worth stressing that this is purely a performance issue — apart from the LORRI import break in my earlier comment, results on this branch match main to ~1e-10 on everything I measured.

[markshowalter]

Regarding "2. Unregistered frames are retained for the life of the process", we can look into whether a strict upper limit on the _WAYFRAMES dicts is needed. However, I don't see a real-world situation that needs 10,000 C matrices. If you are iteratively testing C matrices in your navigation procedure, the correct solution is to build a Navigation frame that operates atop your Cmatrix frame. The Navigation frame is Fittable, so if the Observation's frame is a Navigation, you can iteratively apply set_params() to it till you find your best-fit solution. That's how it's supposed to work. There's probably a need to update the Cassini ISS class to allow for such a frame to exist, but in the mean time I think obs.frame = Navigation((0,0), obs.frame) would work. Then any update to the frame would change the pointing without creating a brand new Cmatrix object.

Check out hosts/jwst/nircam for an example of navigation as in input parameter to the constructor. Obviously, having a unified set of host modules is needed so that we don't need to implement the navigation input for every host subclass.

Until we get there, I think doing nothing is a reasonable approach to this issue, sinceit just wastes a bit of core memory, nothing more. If memory really is an issue, we could implement a workaround, such as limiting the size of the Cmatrix._WAYFRAMES cache, but I would prefer not.

BTW, the _WAYFRAMES caches are there to make sure that if you define an identical object multiple times, they are all equivalent. It's important for the __setstate__, __getstate__ mechanism.

[markshowalter]

"1. oops.hosts.newhorizons.lorri cannot be imported" is fixed in this checkin. "3. Two behavior/API changes that deserve a line in the PR description" is addressed by a few words in the PR description.

The performance issue will take a little bit of investigation to determine whether the old or new behavior is correct. Specifically, what should happen is that the SpiceFrame uses a QuickFrame but the RingFrame should not. RingFrame + SpiceFrame should invisibly be implemented as RingFrame + QuickFrame, not as its own unique QuickFrame.

[markshowalter]

Next point about timing. You should be using RingFrame(epoch=0.) for Saturn, because the rotation pole is (essentially) fixed. That means that the RingFrame, which is a "despun" version of the planet's PCK frame, is inertial. So by fixing the epoch at any time, you will be devoting roughly zero time to the evaluation of this frame.

I see the timing issue you raised, and am looking into the best solution. But still, this is time you could be saving.

[jnspitale]

Raising an interaction with #201, following @rfrenchseti's note there (#201 (comment)): as they describe it, the registry rewrite here inserts every frame into a per-subclass _WAYFRAMES table regardless of frame_id, dedups equal-valued unregistered frames to a single shared wayframe, and retains each construction for the life of the process.

Two consequences for Observation.set_cmatrix(frame_id=None) (added in #201):

1. Retention. The frame_id=None path exists so batch consumers (SpinDoctor loading thousands of images, each with its own corrected pointing) get one throwaway frame per observation with no global footprint. If every unregistered frame is retained process-lifetime, that's an unbounded leak proportional to images loaded — independent of the cmatrix API, this affects any code constructing anonymous frames in a loop. If the new oops/cache.py LRU bounds this, that may already be the answer; worth confirming _WAYFRAMES is under it.

2. Dedup. Sharing one wayframe between equal-valued immutable frames doesn't corrupt pointing (re-pointing builds a new frame with different values), but it does falsify Add custom C-matrix support via generic Observation.get_cmatrix/set_cmatrix #201's documented contract that each observation owns its frame. And under the Fittable/mutable framework introduced here, "immutable" may no longer hold: if a shared frame object can later be adjusted in place via set_params, dedup turns per-observation pointing into aliased pointing — which would be a real bug, not just a doc issue.

#201 now pins its contract with a test (test_default_frames_are_distinct_and_stay_out_of_registry: equal-valued C-matrices → distinct frame objects; registry/cache sizes unchanged by a default load), so whichever branch merges second will see a concrete failure rather than a silent semantic change. Flagging it now so the reconciliation can be a design decision here rather than a rebase surprise.

[markshowalter]

The timing issue should be resolved now. Please check. And I really do think you should revisit how you define the RingFrame, as I mentioned earlier, because it should really be treated as an inertial frame for Saturn. Neptune might be a different case because of its polar wander, but even that is very small over the time frame of the Voyager flyby, so you could get by with defining a RingFrame for the epoch of the flyby in that case.

We are going to need to make some changes to the set_cmatrix/get_cmatrix PR. I left a review explaining what the issues are. Claude's objection to the "dedup" of C matrices is resolved by the Fittable interface, but I would prefer not to make a Cmatrix Fittable, and recommend using the Navigation API instead. That's exactly what it's designed for. I think the issues are easily resolved, but will affect how SpinDoctor calls OOPS during navigation.

[rfrenchseti]

It will change how I use oops during navigation, or how I use oops to get the final cmatrix? I don’t see how I can change the navigation process. It’s done on an original, non-offset image and associated backplanes, and that isn’t going to change. The final offset is in pixels. I then have to convert the pixels to an offset frame and a new cmatrix. That fundamental process can’t change.

[markshowalter]

It sounds like you are using something morally equivalent to an OffsetFOV. This is also Fittable, and the units are pixels. It does not currently support rotations, but that would be an easy modification. It sounds like this is what you are doing, but without using this API.

[markshowalter]

I think this checkin now addresses the remaining issues. Claude repaired a few more bugs.

In addition, this addresses my concerns about #201. I took Joe's changes, merged them into mine, and let Claude proceed. There is now a general new Observation method set_frame(), which replaces an Observation's frame while correctly allowing any revisions to propage to any other object that depends on the Observation, such as a Backplane. The internal caches of the Backplane reset automatically when the Observation changes, and this should all be invisible to the user.

There are also two methods set_spice_cmatrix() and get_spice_cmatrix(), which do what is expected.

The Cassini ISS implementation is slightly different from what Joe submitted. Observation gets an optional subfield "spice_to_cmatrix", a Matrix3 that rotates the SPICE C matrix to the OOPS version. If the Observation has this subfield, then those two new methods work; otherwise, it doesn't. I think this is cleaner than saving the entire host's class object inside the Observation.

[jnspitale]

Looks good overall to me, but what is the justification for assuming the mid time instead of including it as an arg?