Phase 9: per-instrument camera-rotation correction#121
Merged
Conversation
VGISS and GOSSI flipped to fit_camera_rotation=true. Every technique now produces a 3x3 covariance and a populated rotation_rad / sigma_rotation_rad pair when the per-image flag is on; the ensemble's pinvh-based combine fuses 3-DoF results uniformly. DT techniques run rotation as a third LM parameter via the existing lm_subpixel_refine machinery. BodyDiscCorrelateNav runs the 11+5+3 rotation-sample pyramid; star techniques run weighted Kabsch / Procrustes (StarFieldFromCatalogNav N-point, StarUniqueMatchNav 2-star, StarRefineNav N-point); centroid / 1-star paths report a rank-deficient 3x3 with the ROTATION_UNOBSERVABLE_VARIANCE sentinel. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
Warning Rate limit exceeded
To keep reviews running without waiting, you can enable usage-based add-on for your organization. This allows additional reviews beyond the hourly cap. Account admins can enable it under billing. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (11)
WalkthroughThis PR implements Phase 9 ("Camera rotation correction per instrument") by adding optional in-plane camera rotation estimation alongside translation. Rotation fitting is configurable per-instrument via Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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. Review rate limit: 0/1 reviews remaining, refill in 24 minutes and 59 seconds.Comment |
Collaborator
Author
|
@CodeRabbit review |
✅ Actions performedReview triggered.
|
Addresses the post-merge review findings: - Add ``ROTATION_AT_EDGE_FRACTION`` shared constant in ``nav.nav_technique.nav_technique`` and use it in every 3-DoF technique instead of the inline ``0.95`` magic number. - Hoist every inline ``ROTATION_UNOBSERVABLE_VARIANCE`` / ``embed_rotation_unobservable`` import in star techniques and tests to the module-level import block. - ``embed_rotation_unobservable`` now validates input shape; ``similarity_transform_fit`` rejects negative weights up front. - Reconcile the AUTONAV_PLAN snapshot list with the Phase 9 ``(complete)`` heading; reword the curator-precision note to reference ``CONFIDENCE_DECIMALS`` instead of inlining "3 decimals". - DT techniques now reuse the already-computed ``sigma_rotation_rad`` for the rotation INFO line, validate covariance shape before any array access, log a warning when ``lm_subpixel_refine`` returns an unexpected shape with ``fit_rotation=False``, and guard the navigate path against an empty vertex aggregation. - ``BodyDiscCorrelateNav`` raises on a non-(2,2) NCC covariance, guards ``_composite_pivot_vu`` against an empty feature list, drops the unused ``winner`` argument from ``_rotation_sigma_from_quality``, and corrects the level-0 sample- step description (the ``np.linspace`` step is ``2 * max_rotation_deg / 10``, exactly 1 deg only at the default). - Expanded Google-style docstrings for ``rotation_pivot_distance_px``, ``rotation_unobservable_sigma_rad``, ``embed_rotation_unobservable``, ``BodyBlobNav._fail_no_signal``, ``BodyDiscCorrelateNav._ncc_at_rotation``, ``BodyDiscCorrelateNav._evaluate_rotation_samples``, ``BodyDiscCorrelateNav._rotation_sigma_from_quality``, ``StarUniqueMatchNav._similarity_fit_assignment``, ``StarUniqueMatchNav._build_two_star_covariance_3dof``, and the ``_rotate_about`` test helper. - Tests: replace vacuous ``assert ... if hasattr(...)`` with a direct status check, tighten rotation tolerances toward the expected zero-rotation outcome, replace fragile ``> 0`` checks with ``pytest.approx`` against the analytic combined sigma, refactor ``_minimal_context`` to accept kwargs so ``test_navcontext_rotation_fields_propagate`` no longer rebuilds the dataclass, hoist ``import math`` / ``import pytest`` in test files, and add the missing ``spurious is False`` / Procrustes-translation assertions to the 3-DoF star unique-match tests. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Each 3-DoF technique now reads ``rotation_at_edge_fraction`` from its ``config_510_techniques.yaml`` tuning block (default 0.95) instead of referencing the module-level ``ROTATION_AT_EDGE_FRACTION`` constant directly. The constant stays as the documented default value that ships in the YAML, parallel to ``at_edge_tolerance_px``. Phase 10 calibration can now retune the rotation-edge fraction per technique without touching code; ``BodyDiscCorrelateNav`` gains its first ``tuning`` block to carry the new key. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 16
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/nav/nav_orchestrator/ensemble.py (1)
504-514:⚠️ Potential issue | 🟠 MajorAdd
rotation_radandsigma_rotation_radparameters toNavResult.conflicted()and thread them through in the ensemble conflicted branch.The
NavResult.ok()builder accepts and forwards rotation fields, butNavResult.conflicted()does not include them in its signature. If a 3-DoF ensemble reaches the conflicted branch (lines 504–514), thecombined.rotation_radand computedsigma_rotation_radare discarded, leaving a 3×3 covariance without the required rotation metadata. This violates the contract enforced in_result_param_vector()(line 136–138), which requires 3×3 covariance to haverotation_radset.Update
NavResult.conflicted()to acceptrotation_rad: float | None = Noneandsigma_rotation_rad: float | None = None, pass them to the returned instance, and forward them in the ensemble conflicted branch. Add a regression test for 3-DoF conflicted outcomes.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/nav/nav_orchestrator/ensemble.py` around lines 504 - 514, NavResult.conflicted currently ignores rotation metadata; update its signature to accept rotation_rad: float | None = None and sigma_rotation_rad: float | None = None, store these on the returned NavResult instance (same as NavResult.ok does), and ensure the ensemble conflicted branch (where NavResult.conflicted(...) is returned around combined.offset_px) forwards combined.rotation_rad and the computed sigma_rotation_rad into those new parameters; also add a regression test asserting a 3-DoF conflicted outcome produces a 3×3 covariance and non-None rotation_rad/sigma_rotation_rad and that _result_param_vector() no longer raises for this case.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/developer_guide_rotation.rst`:
- Around line 167-174: The docs currently state the rotation-edge threshold as a
hard-coded 0.95 * max_rotation_deg; update the text to reference the
configurable tuning parameter by describing the condition as
rotation_at_edge_fraction * max_rotation_deg (where rotation_at_edge_fraction is
read from each technique's tuning, defaulting to 0.95), and keep the rest of the
sentence about OR-ing with translation at_edge and the INFO log for
fit_camera_rotation and NavTechniqueResult.at_edge unchanged so the guide
matches the implementation.
- Around line 129-131: The docs incorrectly label ROTATION_UNOBSERVABLE_VARIANCE
as having units "px²"; update the documentation text for the constant
ROTATION_UNOBSERVABLE_VARIANCE to state the correct units "rad²" (angular
variance) and clarify that it occupies the rotation slot of the 3x3 covariance
matrix so readers understand it's an angular variance sentinel used in rotation,
not a pixel variance. Ensure the amended sentence replaces "px²" with "rad²" and
retains the note about it being a large finite sentinel for +inf used with
np.linalg.eigvalsh.
In `@src/nav/nav_orchestrator/nav_context.py`:
- Around line 86-87: NavContext now exposes fit_camera_rotation and
max_rotation_deg but lacks validation; add a __post_init__ on class NavContext
that (1) ensures fit_camera_rotation is a bool (raise TypeError if not), (2)
ensures max_rotation_deg is a finite number (int/float) and >= 0 (raise
TypeError for wrong type, ValueError for negative or NaN/inf), and (3) coerce
ints to float for max_rotation_deg if valid; reference the NavContext class, the
fit_camera_rotation and max_rotation_deg attributes, and implement checks and
raises inside __post_init__ to enforce the public API contract.
In `@src/nav/nav_technique/nav_technique_body_blob.py`:
- Around line 399-406: Update the navigate() docstring to describe the new
rotation-aware return format: explain that the function can now return either a
2x2 or 3x3 covariance matrix (depending on whether rotation is fit), that
rotation_rad and sigma_rotation_rad may be present (or None) on the result, and
clarify how confidence, spurious, at_edge, diagnostics, and covariance_px2 are
represented in both cases; also mirror this docstring change where similar
documentation appears around the other instance noted (lines ~451-453) so both
places accurately describe the 2x2 vs 3x3 behavior and the optional rotation
fields.
In `@src/nav/nav_technique/nav_technique_body_disc.py`:
- Around line 448-467: The sampling can exceed ±max_rotation_deg and sigma is
computed at the middle sample regardless of which sample won; update the
sample-generation and sigma computation: when building l1_thetas and l2_thetas
(and the analogous block at lines ~627-644) clamp each candidate theta to lie
within +/- math.radians(max_rotation_deg) from the nominal base (e.g.,
l0_winner.theta_rad or l1_winner.theta_rad) before passing to
_evaluate_rotation_samples, and change _rotation_sigma_from_quality usage to
compute sigma around the actual winner index (use the winner's rank/index and
fit curvature using that winner and its immediate neighbors) rather than always
using the middle sample so the returned sigma corresponds to the returned theta.
- Around line 109-132: The current use of np.roll (around variables
shift_v/shift_u on template_img/template_mask) causes circular wraparound;
replace each np.roll-based shift with explicit zero-filled translation: compute
source and destination slice ranges from shift_v/shift_u, create zero arrays
matching template_img/template_mask, copy only the overlapping window
(respecting dtypes—float for template_img, uint8 for template_mask) into the
destination so out-of-bounds pixels are dropped rather than wrapped, then pass
those zero-padded arrays to ndimage_rotate (same angle=math.degrees(theta_rad)
and order/mode settings) and finally reverse the translation after rotation
using the same crop/pad logic to realign rotated and rotated_mask with the
original frame.
In `@src/nav/nav_technique/nav_technique_body_limb.py`:
- Around line 262-282: Update the navigate() docstring to reflect the new return
contract: document that when fit_rotation is True the method returns
rotation_rad and sigma_rotation_rad and a (3, 3) covariance
(translation+rotation), and when fit_rotation is False it returns a (2, 2)
covariance with rotation_rad and sigma_rotation_rad set to None; mention types
(float | None) for rotation_rad and sigma_rotation_rad and clarify covariance
shape behavior and truncation (covariance[:2,:2]) so callers know to handle both
2x2 and 3x3 covariance outputs.
In `@src/nav/nav_technique/nav_technique_body_terminator.py`:
- Around line 281-301: The docstring for navigate() on BodyTerminatorNav is
stale: update its documented return contract to reflect the new rotation-aware
output (when fit_rotation=True) which now returns rotation_rad and
sigma_rotation_rad and a (3, 3) covariance; explicitly document both modes
(fit_rotation=True -> 3-DoF: position, rotation_rad: float, sigma_rotation_rad:
float, covariance (3,3); fit_rotation=False -> 2-DoF: position, covariance (2,2)
and rotation fields as None). Mention types and when rotation fields are None,
and update any examples and type hints in the navigate() docstring accordingly.
In `@src/nav/nav_technique/nav_technique_ring_annulus.py`:
- Around line 211-217: The navigate() docstring still claims it returns only 2x2
covariance but the code conditionally returns a 3x3 covariance and a rotation
field when context.fit_camera_rotation is true; update the navigate() docstring
to state that the returned 'covariance' may be 2x2 or 3x3 depending on
fit_camera_rotation and that a 'rotation' (or rotation-related) field is
included in the 3-DoF case; also update the second docstring block around the
other navigate-related return docs (the other docstring referenced near the
covariance handling) to match this 2-DoF vs 3-DoF contract and mention the shape
and types for both branches.
In `@src/nav/nav_technique/nav_technique_star_field.py`:
- Around line 715-719: The INFO log currently appends ', AT_EDGE' only when
rotation_at_edge is true but not when the translation search hit the boundary
(at_edge), so update the logging in the self.logger.info call that prints
Rotation (the call referencing rotation_rad, sigma_rotation_rad,
rotation_at_edge) to also include the marker when at_edge is true (i.e., use ',
AT_EDGE' if rotation_at_edge or at_edge else ''), or alternatively ensure the
offset/translation INFO line (the one logging offset_x/offset_y or similar)
appends ', AT_EDGE' when at_edge is true so both rotation- and translation-edge
results consistently show the AT_EDGE suffix.
In `@src/nav/nav_technique/nav_technique_star_refine.py`:
- Around line 272-306: The branch that calls _fit_rotation_3dof updates
offset_v_total and offset_u_total but doesn't recompute at_edge, leaving it
based on the pre-refit delta+prior; after the call to _fit_rotation_3dof (inside
the if fit_rotation block) recompute at_edge using the updated offset_v_total
and offset_u_total with the same search-margin check used earlier (i.e., compare
the updated offset magnitude against the search margin threshold that was used
to set at_edge previously), and then use that recomputed at_edge value for the
subsequent logic and logging (affecting the rotation_at_edge flag and AT_EDGE
marker).
In `@src/nav/nav_technique/nav_technique_star_unique_match.py`:
- Around line 185-189: Update the docstring for the return value in
nav_technique_star_unique_match (specifically describing NavTechniqueResult from
_try_two_star and the 1-star path) to separately document the 1-star and 2-star
cases: state that the 1-star case returns offset, a 2x2 covariance (rotation
unobservable), calibrated confidence, and StarUniqueMatchDiagnostics; state that
the 2-star case now returns offset plus an observed rotation (rotation_rad), its
uncertainty (sigma_rotation_rad), and a full 3x3 covariance reflecting rotation
observability, along with calibrated confidence and populated
StarUniqueMatchDiagnostics; ensure the wording matches the actual returned
fields (rotation_rad, sigma_rotation_rad, 2x2 vs 3x3 covariance) and mention the
fit_camera_rotation behavior only where still applicable.
In `@src/nav/nav_technique/nav_technique.py`:
- Around line 97-103: The helper embed_rotation_unobservable currently only
validates shape; update it to also reject non-finite covariance entries by
checking np.isfinite on the converted array (arr or cov_2x2) and raise a clear
ValueError if any element is NaN/inf; keep the existing shape check, then
perform the finiteness check before building out and assigning
ROTATION_UNOBSERVABLE_VARIANCE so callers get an immediate, explicit error
instead of downstream linear-algebra failures.
In `@tests/nav/nav_technique/test_nav_technique_body_limb.py`:
- Around line 343-363: The test currently forges a fixed 4.9° rotation which
hardcodes the old 0.95*5.0 cutoff; update the test to derive the forged angle
from the technique tuning by reading rotation_at_edge_fraction from
BodyLimbNav.tuning and multiplying by the local max_rotation_deg (or the test's
max_rotation_deg constant) and use that product converted to radians for
LMRefineResult.rotation_rad and the assert; adjust the monkeypatch/forged_result
construction to compute rotation_rad = np.deg2rad(rotation_at_edge_fraction *
max_rotation_deg) so future retunes won't break the test.
---
Outside diff comments:
In `@src/nav/nav_orchestrator/ensemble.py`:
- Around line 504-514: NavResult.conflicted currently ignores rotation metadata;
update its signature to accept rotation_rad: float | None = None and
sigma_rotation_rad: float | None = None, store these on the returned NavResult
instance (same as NavResult.ok does), and ensure the ensemble conflicted branch
(where NavResult.conflicted(...) is returned around combined.offset_px) forwards
combined.rotation_rad and the computed sigma_rotation_rad into those new
parameters; also add a regression test asserting a 3-DoF conflicted outcome
produces a 3×3 covariance and non-None rotation_rad/sigma_rotation_rad and that
_result_param_vector() no longer raises for this case.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 8d55b64d-a72f-445e-90f7-790e7aba2b47
📒 Files selected for processing (32)
AUTONAV_PLAN.mddocs/developer_guide.rstdocs/developer_guide_rotation.rstsrc/nav/config_files/config_410_inst_gossi.yamlsrc/nav/config_files/config_430_inst_vgiss.yamlsrc/nav/config_files/config_510_techniques.yamlsrc/nav/nav_orchestrator/ensemble.pysrc/nav/nav_orchestrator/nav_context.pysrc/nav/nav_orchestrator/orchestrator.pysrc/nav/nav_technique/_star_helpers.pysrc/nav/nav_technique/nav_technique.pysrc/nav/nav_technique/nav_technique_body_blob.pysrc/nav/nav_technique/nav_technique_body_disc.pysrc/nav/nav_technique/nav_technique_body_limb.pysrc/nav/nav_technique/nav_technique_body_terminator.pysrc/nav/nav_technique/nav_technique_ring_annulus.pysrc/nav/nav_technique/nav_technique_ring_edge.pysrc/nav/nav_technique/nav_technique_star_field.pysrc/nav/nav_technique/nav_technique_star_refine.pysrc/nav/nav_technique/nav_technique_star_unique_match.pytests/nav/config_files/test_config_load.pytests/nav/nav_orchestrator/test_ensemble.pytests/nav/nav_orchestrator/test_nav_context.pytests/nav/nav_technique/conftest.pytests/nav/nav_technique/test_nav_technique_body_blob.pytests/nav/nav_technique/test_nav_technique_body_disc.pytests/nav/nav_technique/test_nav_technique_body_limb.pytests/nav/nav_technique/test_nav_technique_body_terminator.pytests/nav/nav_technique/test_nav_technique_ring_edge.pytests/nav/nav_technique/test_nav_technique_star_field.pytests/nav/nav_technique/test_nav_technique_star_refine.pytests/nav/nav_technique/test_nav_technique_star_unique_match.py
Addresses the second round of review findings on PR #121: - ``_rotate_template`` (BodyDiscCorrelateNav) replaces the np.roll shift with a zero-padded translate via the new ``_zero_padded_shift`` helper so out-of-bounds pixels drop instead of wrapping to the opposite edge. Wraparound was a theoretical bug for body templates whose support sits anywhere off the array centre — the second shift back would smear template content across the image after rotation. - The level-1 and level-2 rotation samples are clamped to ``[-max_rotation_rad, +max_rotation_rad]`` so the outer-loop search never proposes a rotation outside the operator-supplied envelope. - ``_rotation_sigma_from_quality`` regains the ``winner`` parameter and returns ``None`` when the level-2 winner is at one of the side samples (the 3-sample finite-difference parabola is only valid when the winner is at the centre). Edge-winners fall back to the rotation-unobservable sentinel rather than report a curvature taken at the wrong centre. - Star-refine recomputes ``at_edge`` after the Procrustes refit since the refit can pull the absolute offset outside the search window even when the delta-based check was False. - Star-field's rotation INFO log now annotates ``AT_EDGE`` when either the rotation cap or the translation margin trips the flag, matching the joint behaviour of the result's ``at_edge`` field. - BodyBlobNav, BodyLimbNav, BodyTerminatorNav, RingAnnulusNav, and StarUniqueMatchNav navigate / _fail_no_signal docstrings now describe the 2-DoF vs 3-DoF return contract: covariance shape, ``rotation_rad`` / ``sigma_rotation_rad`` semantics, and the rank-deficient-rotation behaviour where applicable. - ``SimilarityFit`` docstring corrected: the implementation stores ``dc - R @ cc`` (global-frame translation), not the pivot-aware ``R(c - pivot) + pivot + translation`` form the previous text described. The pivot is reported alongside so callers can recover the pivot-relative translation explicitly. - ``developer_guide_rotation.rst`` references the configurable ``rotation_at_edge_fraction`` instead of a hardcoded ``0.95``. - ``test_body_limb_nav_3dof_at_edge_when_rotation_saturates`` derives the forged rotation angle from ``BodyLimbNav.tuning['rotation_at_edge_fraction']`` so future retunes do not break the test. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Purpose
Phases 4–8 produced a 2-DoF (translation-only) baseline. Voyager ISS and Galileo SSI carry per-image attitude rotation residuals up to several degrees that cannot be calibrated out mission-wide, so the new pipeline must fit a small camera rotation alongside the per-image translation on those instruments. Phase 9 enables that knob, threads 3-DoF math through every technique and the ensemble combine, flips the per-camera flag on for VGISS / GOSSI, and keeps Cassini ISS / NHLORRI on the existing 2-DoF path. Closes the Phase 9 work item in
AUTONAV_PLAN.md.Changes
config_410_inst_gossi.yamlandconfig_430_inst_vgiss.yamlflipfit_camera_rotation: true; Cassini ISS and NHLORRI stay false. The orchestrator readsfit_camera_rotationandmax_rotation_degviainstrument_settings_from_obsand plumbs them onto a frozenNavContext.NavContext.with_priornow accepts either a 2x2 or a 3x3 prior covariance and projects to the 2x2 translation block before pass 2 (rotation prior carries no useful information across the pass boundary).BodyLimbNav,BodyTerminatorNav,RingEdgeNav) threadfit_rotationthrough the existinglm_subpixel_refine(vertex-centroid pivot; rotation cap drivesat_edge).BodyDiscCorrelateNavruns the full 11 + 5 + 3 rotation-sample pyramid (scipy.ndimage.rotate-based template pre-rotation about the centroid-of-body-centres pivot; level-2 NCC quality curvature feedssigma_theta). Star techniques run weighted Kabsch / orthogonal Procrustes via the new_star_helpers.similarity_transform_fit:StarFieldFromCatalogNavN-point Tukey-reweighted,StarUniqueMatchNav2-star exact,StarRefineNavN-point on inliers.BodyBlobNav,RingAnnulusNav, and 1-star / 1-inlier paths report a rank-deficient 3x3 (translation observable, rotation unobservable) viaembed_rotation_unobservablecarrying theROTATION_UNOBSERVABLE_VARIANCE = 1.0e15finite sentinel (a literal+infwould makenp.linalg.eigvalshreturn NaN)._combine_precision_weightedreturns a typed_CombinedEstimatedataclass;_result_param_vectordispatches on covariance shape so(dv, du)and(dv, du, theta)parameter vectors flow through Mahalanobis grouping, single-link clustering, and the precision-weighted information-form merge uniformly. Mixed-DoF inputs raise a recognisableValueError(the orchestrator pins the DoF per image so this fires only on a programmer error). Rotation propagates ontoNavResult.ok/NavResult.conflictedoutputs.rotation_radis populated, the curator emitsrotation_degandsigma_rotation_degrounded to the canonical curator confidence-precision; both fields are omitted entirely when the flag is off, so 2-DoF runs do not litter the JSON. Tier derivation (max_sigma_px) consults only the translation sigma so a rotation outcome can never inflate a tier.rotation_at_edge_fraction(default 0.95) from itsconfig_510_techniques.yamltuning block, parallel toat_edge_tolerance_px.ROTATION_AT_EDGE_FRACTIONstays innav.nav_technique.nav_techniqueas the documented default that ships in the YAML; Phase 10 calibration can retune per technique without code changes.AT_EDGEannotation when|theta| >= rotation_at_edge_fraction * max_rotation_deg. The translation-sideat_edgerule is unchanged; both contributions are OR-ed ontoNavTechniqueResult.at_edge.docs/developer_guide_rotation.rstcovers the per-instrument flag, parameter vector, pivot rules, per-technique strategy, rank-deficient pattern, ensemble combine, JSON output, andat_edgesemantics. The Phase 9 section ofAUTONAV_PLAN.mdis rewritten as the canonical "complete" record (matching the Phase 8 style) and the snapshot list is updated for consistency.phase_09_review/ships the six per-phase critique files plus an executive summary against.cursor/rules/python_best_practices.mdc,.cursor/rules/documentation.mdc,.cursor/rules/filecache_best_practices.mdc,.cursor/rules/logging_best_practices.mdc, thecritique-test-suiteskill, and thepython-codebase-analysisskill.Type of Change
(Phase 9 is primarily a new feature; the docs and tests checkboxes are checked because the change-set ships material in both areas alongside the production code.)
Testing
pytest -n auto --dist=loadfilereports 1230 / 1230 passing;pytest --covretains the existing per-package coverage thresholds. Nineteen new unit tests cover the new behaviour:NavContext.with_prior; rotation-field defaults and propagation.BodyLimbNav(3-DoF result + rotationat_edge),BodyTerminatorNav,RingEdgeNav(curved + flat-edge rank-deficient),BodyDiscCorrelateNav,BodyBlobNav(rank-deficient sentinel),StarFieldFromCatalogNav(planted rotation recovery + zero-rotation path),StarUniqueMatchNav(2-star Procrustes + 1-star unobservable),StarRefineNav(multi-inlier Procrustes + single-inlier unobservable).Manual verification:
./scripts/run-all-checks.shis green end-to-end (ruff check,ruff format --check,mypy --strict,pytest -n auto --dist=loadfile,sphinx-build -W -b html docs docs/_build,pymarkdown scan docs/ .cursor/ README.md CONTRIBUTING.md).Integration / end-to-end test boxes are unchecked: integration tests are skipped by default in this branch's CI matrix (
pytest -m "not integration"); the existing 2-DoF integration suite is unaffected because the new code paths only execute whenfit_camera_rotationis True. A real-image VGISS / GOSSI library expansion is intentionally deferred to Phase 10 (calibration) perAUTONAV_PLAN.md.Potential Impacts
NavContextgains two fields (fit_camera_rotation: bool = False,max_rotation_deg: float = 5.0), both with safe defaults so existing constructors that omit them keep the 2-DoF behaviour.NavResult.ok(...)already acceptedrotation_rad/sigma_rotation_rad(shipped in Phase 0); this change populates them. New helpers (ROTATION_AT_EDGE_FRACTION,ROTATION_UNOBSERVABLE_VARIANCE,embed_rotation_unobservable,rotation_pivot_distance_px,rotation_unobservable_sigma_rad) are added tonav.nav_technique.nav_technique.__all__._combine_precision_weightedreturns a_CombinedEstimatedataclass instead of a tuple — internal helper (leading underscore), no documented external consumers.fit_camera_rotation: false; their navigation results carry 2x2 covariances and omit rotation fields exactly as before. Voyager ISS / Galileo SSI configs flip on and now produce 3x3 covariances with rotation entries; downstream consumers (backplanes, PDS4 bundles) read translation only and are unaffected.BodyDiscCorrelateNavruns ~ 19x the 2-DoF baseline (11 + 5 + 3 NCC pyramids) whenfit_camera_rotationis on; this only applies to instruments where the flag is true (VGISS / GOSSI), and body-disc scenes are uncommon there. Cassini / NHLORRI compute is unchanged.nav_techniqueornav_orchestratorinternals. The_metadata.jsonschema gains optionalrotation_deg/sigma_rotation_degkeys (omitted entirely when the flag is off), so existing JSON readers stay forward-compatible.Checklist
ruff check,ruff format)mypypasses(
CHANGES.mdis not present in this repository; the canonical user-facing record for the cutover work lives inAUTONAV_PLAN.mdplus the per-phasephase_NN_review/folders.)Notes
AUTONAV_PLAN.md) is intentionally deferred to Phase 10 alongside the wider library expansion to ~50 images and the one-time confidence-formula calibration. Phase 9 ships the math, the tunables, and the per-instrument flag flips; Phase 10 turns those knobs into calibrated numbers against ground truth.LimbPolyline/TerminatorPolyline/RingEdgePolylinedo not currently carry the body centre and the LM fit is robust to small pivot mis-placement; (2) the integer-rounded pivot shift in_rotate_template(BodyDiscCorrelateNav) sacrifices < 1 px of pivot accuracy for a much simpler implementation, well below the 0.25 deg level-2 sample step.🤖 Generated with Claude Code