mirror of
https://github.com/azaion/gps-denied-onboard.git
synced 2026-06-22 23:21:13 +00:00
[AZ-271] [AZ-276] [AZ-278] [AZ-282] Finish cross-cutting helpers + relax opencv pin
E-CC-HELPERS closes with the three remaining Layer-1 helpers and E-CC-CONF closes with the env > YAML > defaults precedence test gate. All four tickets ship with frozen public surfaces, hermetic unit tests, and no upward (components.*) imports. * AZ-271 — tests/unit/shared/config/test_precedence.py (5 ACs + smoke test + helper that names the layer in failure messages). * AZ-282 — helpers/ransac_filter.py: static RansacFilter + RansacResult; cv2.setRNGSeed(0) for byte-equal determinism; median residual semantics pinned by contract. * AZ-276 — helpers/imu_preintegrator.py + make_imu_preintegrator; GTSAM PreintegratedCombinedMeasurements; strict-monotonic ts_ns guard runs before any state mutation. Adjacent hygiene: _types/nav.py ImuSample/ImuWindow now use ts_ns:int and the spec-mandated ImuBias dataclass. * AZ-278 — helpers/lightglue_runtime.py: structural R14 fix. LightGlueRuntime + non-blocking concurrent-access guard that raises rather than serialising. EngineHandle Protocol in _types/manifests.py + KeypointSet/CorrespondenceSet in _types/matching.py (Protocol surface adds approved by spec). Dependency conflict (Finding 1, user-approved): gtsam 4.2 (PyPI) is numpy-1.x-ABI only; opencv-python>=4.12 needs numpy>=2 at runtime. Resolution: opencv-python pin relaxed to >=4.11.0.86,<4.12. The D-CROSS-CVE-1 ratchet at ci/opencv_pin_gate.py is held at 4.11.0 with the original 4.12.0 floor restored once a numpy-2-compatible gtsam wheel ships. Full replay procedure in _docs/_process_leftovers/2026-05-11_d_cross_cve_1_opencv_pin_deferred.md. Tests: 294 passed, 2 skipped (cmake/actionlint env-skips, pre-existing). 43 new tests added for batch 5. Ruff check + format clean. Co-authored-by: Cursor <cursoragent@cursor.com>
This commit is contained in:
Oleksandr Bezdieniezhnykh
@@ -1,75 +0,0 @@
|
||||
# Config Precedence Unit Tests
|
||||
|
||||
**Task**: AZ-271_config_precedence_tests
|
||||
**Name**: Config Precedence Tests
|
||||
**Description**: A focused unit-test module that verifies the env > YAML > defaults precedence rule for at least 3 keys per layer (per epic AZ-246 AC-3) and the multi-file YAML merge order (later wins). Companion to AZ-269 / AZ-270.
|
||||
**Complexity**: 2 points
|
||||
**Dependencies**: AZ-269_config_loader, AZ-270_compose_root
|
||||
**Component**: shared.config (cross-cutting; epic AZ-246 / E-CC-CONF)
|
||||
**Tracker**: AZ-271
|
||||
**Epic**: AZ-246 (E-CC-CONF)
|
||||
|
||||
### Document Dependencies
|
||||
|
||||
- `_docs/02_document/contracts/shared_config/composition_root_protocol.md` — the contract whose precedence invariant this test suite verifies.
|
||||
|
||||
## Problem
|
||||
|
||||
The composition_root_protocol contract declares a hard precedence rule. Without explicit, tabular tests covering at least 3 keys per layer, regressions in the loader silently flip precedence and cause field bugs that only show up in production deployments where env overrides matter most.
|
||||
|
||||
## Outcome
|
||||
|
||||
- A pytest module with clearly-named cases covering precedence for ≥3 keys at each layer.
|
||||
- A multi-file YAML merge case proving later paths win over earlier paths.
|
||||
- Failure messages name the layer (env / YAML / defaults) so on-call engineers can triage fast.
|
||||
|
||||
## Scope
|
||||
|
||||
### Included
|
||||
|
||||
- Precedence cases for ≥3 keys at each of: env > YAML, YAML > defaults, multi-file YAML merge order.
|
||||
- One reachability case driving `compose_root` end-to-end with a stub Config to prove the loader and composition functions integrate cleanly.
|
||||
- Test fixtures (in-memory YAML strings + env dict, no real files needed for precedence cases; one tmp_path for the multi-file case).
|
||||
|
||||
### Excluded
|
||||
|
||||
- Performance microbench — owned by AZ-269.
|
||||
- Strategy/build-flag mismatch tests — owned by AZ-270.
|
||||
- Per-component config block tests — owned by each component epic.
|
||||
|
||||
## Acceptance Criteria
|
||||
|
||||
**AC-1: env > YAML for at least 3 keys**
|
||||
Given env sets `LOG_LEVEL`, `FDR_QUEUE_SIZE`, `MAVLINK_BAUD` and YAML sets all three to different values
|
||||
When `load_config(env, [yaml])` runs
|
||||
Then all three resolved values match env
|
||||
|
||||
**AC-2: YAML > defaults for at least 3 keys**
|
||||
Given env is empty and YAML sets `log.level`, `fdr.queue_size`, `mavlink.baud`
|
||||
When `load_config(env, [yaml])` runs
|
||||
Then all three resolved values match YAML (not the documented defaults)
|
||||
|
||||
**AC-3: Defaults apply for at least 3 keys**
|
||||
Given env is empty and YAML omits the three keys
|
||||
When `load_config(env, [yaml])` runs
|
||||
Then all three resolved values match the documented defaults
|
||||
|
||||
**AC-4: Multi-file YAML — later wins**
|
||||
Given two YAML paths setting the same key to different values
|
||||
When `load_config(env, [first, second])` runs
|
||||
Then the resolved value matches the second file
|
||||
|
||||
**AC-5: Failure messages name the layer**
|
||||
Given a precedence assertion fails
|
||||
When pytest reports the failure
|
||||
Then the assertion message names which layer's value was expected and which was found
|
||||
|
||||
## Non-Functional Requirements
|
||||
|
||||
**Reliability**
|
||||
- Tests are hermetic: no real env vars consulted, no real YAML files outside tmp_path.
|
||||
|
||||
## Constraints
|
||||
|
||||
- Test file path is fixed at `tests/unit/shared/config/test_precedence.py` (mirrors the `tests/unit/<component>/` convention from module-layout.md Layout Rule 7 — `shared/config` is the component slug).
|
||||
- Cases use the SAME 3 keys per layer to make the test matrix comparable across layers.
|
||||
@@ -1,138 +0,0 @@
|
||||
# ImuPreintegrator Helper Module
|
||||
|
||||
**Task**: AZ-276_imu_preintegrator
|
||||
**Name**: ImuPreintegrator Helper
|
||||
**Description**: Implement the shared `ImuPreintegrator` helper that wraps GTSAM's `PreintegrationCombinedParams` + `PreintegratedCombinedMeasurements` so C1 (VIO) and C5 (StateEstimator) consume one canonical preintegration of every FC IMU window. Single-threaded by design; one instance per writer thread, bound by the composition root. Bias drift remains the consumer's responsibility.
|
||||
**Complexity**: 2 points
|
||||
**Dependencies**: AZ-263_initial_structure
|
||||
**Component**: shared.helpers.imu_preintegrator (cross-cutting; epic AZ-264 / E-CC-HELPERS)
|
||||
**Tracker**: AZ-276
|
||||
**Epic**: AZ-264 (E-CC-HELPERS)
|
||||
|
||||
### Document Dependencies
|
||||
|
||||
- `_docs/02_document/contracts/shared_helpers/imu_preintegrator.md` — frozen public interface this task produces.
|
||||
- `_docs/02_document/common-helpers/01_helper_imu_preintegrator.md` — design rationale and consumer mapping.
|
||||
|
||||
## Problem
|
||||
|
||||
C1's VIO loop and C5's state estimator both consume the same FC IMU window every keyframe. Without a shared preintegrator:
|
||||
- They drift into two slightly-different integrations of the same physical IMU stream (different sample-rejection rules, different bias-application order).
|
||||
- The GTSAM `CombinedImuFactor` shape that goes into C5's iSAM2 graph diverges from the one C1 uses for its own pose update, breaking the "single source of IMU truth" invariant in `solution.md`.
|
||||
- Per-deployment IMU noise covariances (which live in `CameraCalibration`) get parsed twice, with subtle unit differences.
|
||||
|
||||
## Outcome
|
||||
|
||||
- A single `ImuPreintegrator` is the only path through which any onboard process integrates IMU samples for a GTSAM `CombinedImuFactor`. C1 and C5 import it; nothing else does.
|
||||
- The composition root binds ONE instance per writer thread; the helper's contract test confirms it does not acquire any locks (so no surprise serialisation under load).
|
||||
- Sample monotonicity is enforced — non-monotonic samples raise `ImuPreintegrationError` before any state is mutated.
|
||||
- Re-bias is explicit: `reset_with_bias` is called by consumers when their bias estimate changes; the helper never re-estimates bias internally.
|
||||
|
||||
## Scope
|
||||
|
||||
### Included
|
||||
|
||||
- `ImuPreintegrator` class + factory `make_imu_preintegrator(calibration: CameraCalibration) -> ImuPreintegrator`.
|
||||
- Integration entrypoints: `integrate_sample(ImuSample)`, `integrate_window(ImuWindow)`.
|
||||
- Factor accessors: `current_preintegration() -> CombinedImuFactor`, `reset_for_new_keyframe() -> CombinedImuFactor` (destructive).
|
||||
- Bias control: `reset_with_bias(ImuBias) -> None`.
|
||||
- `ImuPreintegrationError` exception type re-exported alongside the helper.
|
||||
- Re-export of GTSAM's `CombinedImuFactor` (or a thin alias) so consumers do not import GTSAM directly.
|
||||
- Public interface contract published at `_docs/02_document/contracts/shared_helpers/imu_preintegrator.md`.
|
||||
|
||||
### Excluded
|
||||
|
||||
- IMU sample acquisition / FC adapter integration — C8.
|
||||
- Bias estimation / re-bias logic — C1, C5.
|
||||
- Multi-threaded sample feeding — out of contract; helper is single-thread by design.
|
||||
- Serialising preintegrated factors to FDR records — C13.
|
||||
- The ImuSample / ImuWindow / ImuBias DTOs themselves — owned by `_types/nav.py` (AZ-263).
|
||||
|
||||
## Acceptance Criteria
|
||||
|
||||
**AC-1: Round-trip preintegration**
|
||||
Given a synthetic IMU sequence of 100 samples with strictly-monotonic `ts_ns`
|
||||
When the producer calls `integrate_sample` 100 times then `current_preintegration()`
|
||||
Then a `CombinedImuFactor` is returned whose `deltaTij` equals the time span and whose `delta_pose` is non-zero
|
||||
|
||||
**AC-2: Strict monotonicity rejects non-monotonic samples**
|
||||
Given a preintegrator with the last integrated sample at `ts_ns = T`
|
||||
When `integrate_sample(sample)` is called with `sample.ts_ns <= T`
|
||||
Then `ImuPreintegrationError` is raised AND the preintegrator's internal accumulators are unchanged (a subsequent valid sample integrates as if the bad one never came)
|
||||
|
||||
**AC-3: `reset_for_new_keyframe` is destructive**
|
||||
Given a preintegrator with N integrated samples
|
||||
When `reset_for_new_keyframe()` is called
|
||||
Then the returned factor reflects all N samples AND a subsequent `current_preintegration()` (with no further samples) raises `ImuPreintegrationError`
|
||||
|
||||
**AC-4: Re-bias affects subsequent samples only**
|
||||
Given a sequence: `reset_with_bias(bias_a)`, integrate 50 samples, `reset_with_bias(bias_b)`, integrate 50 more
|
||||
When `current_preintegration()` is called
|
||||
Then the resulting factor reflects bias_a applied to samples 1–50 and bias_b applied to samples 51–100 (not bias_b retroactively)
|
||||
|
||||
**AC-5: Determinism**
|
||||
Given two instances constructed from the same calibration and fed the same `(bias, samples)` sequence
|
||||
When both call `current_preintegration()`
|
||||
Then the outputs are deep-equal
|
||||
|
||||
**AC-6: Single-threaded, lock-free**
|
||||
Given the helper's source code
|
||||
When inspected by the contract test (static analysis OR runtime reflection)
|
||||
Then no `threading.Lock`, `RLock`, `Semaphore`, or `mutex` is acquired anywhere in the integration path
|
||||
|
||||
**AC-7: No upward imports (Layer 1 invariant)**
|
||||
Given the helper module
|
||||
When a static-import check runs across `gps_denied_onboard.helpers.imu_preintegrator`
|
||||
Then it imports ONLY from `_types`, GTSAM, numpy, and stdlib — no `gps_denied_onboard.components.*` imports anywhere
|
||||
|
||||
## Non-Functional Requirements
|
||||
|
||||
**Performance**
|
||||
- `integrate_sample` p99 ≤ 200 µs on Tier-2 (Jetson Orin Nano Super) — overhead vs. inline GTSAM PIM ≤ 5 % (per E-CC-HELPERS hot-path NFR).
|
||||
- `current_preintegration` p99 ≤ 100 µs on the same hardware.
|
||||
|
||||
**Reliability**
|
||||
- Pure deterministic: same inputs → byte-equal `CombinedImuFactor` outputs.
|
||||
- `ImuPreintegrationError` is the ONLY exception type the public surface raises on schema/timestamp violation; GTSAM's lower-level exceptions MUST be wrapped.
|
||||
|
||||
## Unit Tests
|
||||
|
||||
| AC Ref | What to Test | Required Outcome |
|
||||
|--------|-------------|-----------------|
|
||||
| AC-1 | 100 monotonic samples → `current_preintegration()` | factor `deltaTij` ≈ time span; non-zero `delta_pose` |
|
||||
| AC-2 | non-monotonic sample injection | `ImuPreintegrationError`; internal state unchanged (next valid sample integrates correctly) |
|
||||
| AC-3 | `reset_for_new_keyframe` then `current_preintegration` | second call raises `ImuPreintegrationError` (state cleared) |
|
||||
| AC-4 | re-bias mid-window | resulting factor distinguishes bias_a vs bias_b epochs |
|
||||
| AC-5 | two instances, same input | deep-equal factor outputs |
|
||||
| AC-6 | static / runtime lock check | no lock acquisition on the integration path |
|
||||
| AC-7 | importlinter / grep gate | no `gps_denied_onboard.components.*` imports |
|
||||
| NFR-perf | microbench `integrate_sample` (10k iterations on Tier-2 fixture) | p99 ≤ 200 µs; overhead ≤ 5 % |
|
||||
|
||||
## Constraints
|
||||
|
||||
- Public surface frozen by `_docs/02_document/contracts/shared_helpers/imu_preintegrator.md` v1.0.0.
|
||||
- Layer 1 Foundation only (per `module-layout.md` § Allowed Dependencies). NO upward imports.
|
||||
- GTSAM is the single math backend — do not introduce a second IMU-preintegration library.
|
||||
- No new dependency beyond what AZ-263 / E-BOOT pinned.
|
||||
|
||||
## Risks & Mitigation
|
||||
|
||||
**Risk 1: Concurrent calls from a misconfigured composition root silently corrupt the GTSAM PIM accumulator**
|
||||
- *Risk*: Two threads call `integrate_sample` simultaneously; GTSAM's PIM is not thread-safe; numerical drift goes undiagnosed.
|
||||
- *Mitigation*: Helper is single-threaded by contract; the composition root binds one instance per writer thread. The contract test (AC-6) asserts no internal locking — making it a hard error if a future change tries to "make it thread-safe" instead of fixing the composition.
|
||||
|
||||
**Risk 2: Sample-monotonicity-rejection silently masks an upstream FC clock skew**
|
||||
- *Risk*: A real IMU stream produces a non-monotonic sample (clock jitter); the helper rejects it; the consumer never learns.
|
||||
- *Mitigation*: `ImuPreintegrationError` carries the offending vs. previous timestamp in its message so the consumer's catch-and-log path can record it as an FDR `kind="imu.skew"` event.
|
||||
|
||||
## Runtime Completeness
|
||||
|
||||
- **Named capability**: GTSAM `CombinedImuFactor` preintegration via `PreintegrationCombinedParams` + `PreintegratedCombinedMeasurements` (architecture / E-CC-HELPERS / `01_helper_imu_preintegrator.md`).
|
||||
- **Production code that must exist**: real GTSAM-backed integration; real noise-model parsing from `CameraCalibration`; real strict-monotonic guard.
|
||||
- **Allowed external stubs**: none — GTSAM is the production runtime.
|
||||
- **Unacceptable substitutes**: pure-numpy "approximate" preintegration that ignores GTSAM's covariance propagation; deterministic-fallback that returns a zero factor; "for now we just integrate position with Euler" placeholder. Each would silently break C5's iSAM2 covariance honesty (AC-NEW-4).
|
||||
|
||||
## Contract
|
||||
|
||||
This task produces the contract at `_docs/02_document/contracts/shared_helpers/imu_preintegrator.md`.
|
||||
Consumers MUST read that file — not this task spec — to discover the interface.
|
||||
@@ -1,146 +0,0 @@
|
||||
# LightGlueRuntime Helper Module (R14 fix)
|
||||
|
||||
**Task**: AZ-278_lightglue_runtime
|
||||
**Name**: LightGlueRuntime Helper
|
||||
**Description**: Implement the shared `LightGlueRuntime` helper that owns the LightGlue inference engine handle for both C2.5 (single-pair inlier counting) and C3 (heavier matching pass). This is the structural fix for R14 (the original C2.5 ↔ C3 import cycle): the runtime sits at Layer 1 with no `components.*` imports, so the cycle becomes impossible to express. Single CUDA stream; concurrent access forbidden by contract; composition root binds to the single F3 hot-path thread.
|
||||
**Complexity**: 3 points
|
||||
**Dependencies**: AZ-263_initial_structure
|
||||
**Component**: shared.helpers.lightglue_runtime (cross-cutting; epic AZ-264 / E-CC-HELPERS)
|
||||
**Tracker**: AZ-278
|
||||
**Epic**: AZ-264 (E-CC-HELPERS)
|
||||
|
||||
### Document Dependencies
|
||||
|
||||
- `_docs/02_document/contracts/shared_helpers/lightglue_runtime.md` — frozen public interface this task produces.
|
||||
- `_docs/02_document/common-helpers/03_helper_lightglue_runtime.md` — design rationale and R14 context.
|
||||
|
||||
## Problem
|
||||
|
||||
C2.5 (Re-rank) and C3 (CrossDomainMatcher) both call LightGlue. In cycle 1 of `_docs/02_document/epics.md`, LightGlue ownership was ambiguous and produced R14: a circular import / runtime dependency between C2.5 and C3 (the "K=10 → N=3 funnel" both wanted to own the engine). Without a shared runtime:
|
||||
- The engine is built / loaded twice, doubling GPU memory at takeoff (Tier-2 has only 8 GB).
|
||||
- C2.5 and C3 drift on engine version pinning, producing inconsistent matches.
|
||||
- Their import cycle is a recurring footgun: any future refactor will tempt one to import from the other.
|
||||
|
||||
## Outcome
|
||||
|
||||
- A single `LightGlueRuntime` instance is constructed once at takeoff by the composition root from C7's `deserialize_engine(LIGHTGLUE_ENGINE_CACHE_ENTRY)` and is constructor-injected into BOTH C2.5 and C3.
|
||||
- The C2.5 ↔ C3 import cycle is structurally impossible: the runtime lives at Layer 1 (`helpers/`) and imports zero `components.*` modules. Both consumers depend on the helper; neither depends on the other.
|
||||
- Concurrent access is rejected at runtime by an explicit guard (`LightGlueConcurrentAccessError`), preserving the single-CUDA-stream invariant. The composition root binds the runtime to the single F3 hot-path thread; AC-4 of the contract is the canary that catches future composition-root mistakes.
|
||||
- The helper exposes no `set_*` / `update_*` methods — once constructed, the runtime's behaviour is fixed.
|
||||
|
||||
## Scope
|
||||
|
||||
### Included
|
||||
|
||||
- `LightGlueRuntime(engine_handle: EngineHandle)` constructor.
|
||||
- `match(features_a: KeypointSet, features_b: KeypointSet) -> CorrespondenceSet` — single-pair path used by C2.5.
|
||||
- `match_batch(features_a_list, features_b_list) -> list[CorrespondenceSet]` — batch path used by C3.
|
||||
- `descriptor_dim() -> int` accessor for shape validation upstream of `match`.
|
||||
- Concurrent-access guard that raises `LightGlueConcurrentAccessError` on overlapping `match` / `match_batch` entries.
|
||||
- `LightGlueRuntimeError` (construction / dim mismatch) and `LightGlueConcurrentAccessError` (concurrent entry) exception types.
|
||||
- Public interface contract published at `_docs/02_document/contracts/shared_helpers/lightglue_runtime.md`.
|
||||
|
||||
### Excluded
|
||||
|
||||
- Engine compilation / serialisation — C7.
|
||||
- Engine filename schema — `helpers.engine_filename_schema` (separate task in this epic).
|
||||
- Engine cache management / takeoff load — C10.
|
||||
- Backbone-specific feature extraction (DISK / ALIKED / XFeat) — C3 / C7.
|
||||
- Multi-GPU / multi-stream / mixed-backbone — out of scope for v1.0.0.
|
||||
- The `EngineHandle` Protocol itself — owned by `_types/manifests.py` (AZ-263) so Layer 1 can reference it without depending on C7.
|
||||
|
||||
## Acceptance Criteria
|
||||
|
||||
**AC-1: Single-pair match (C2.5 path)**
|
||||
Given a pair of `KeypointSet`s with matching descriptor dim and a synthetic-overlap fixture
|
||||
When `match(features_a, features_b)` runs
|
||||
Then a `CorrespondenceSet` is returned with `len > 0` and the inlier-count helper used by C2.5 finds the expected count
|
||||
|
||||
**AC-2: Batch match (C3 path)**
|
||||
Given three pairs of `KeypointSet`s
|
||||
When `match_batch([a1, a2, a3], [b1, b2, b3])` runs
|
||||
Then three `CorrespondenceSet`s are returned in input order; per-pair invariants match the single-pair path
|
||||
|
||||
**AC-3: Descriptor-dim mismatch rejected**
|
||||
Given features whose `descriptor_dim` does not match the engine's expected dim
|
||||
When `match` runs
|
||||
Then `LightGlueRuntimeError` is raised with a message naming both the expected and actual dims
|
||||
|
||||
**AC-4: Concurrent access rejected**
|
||||
Given two threads call `match` simultaneously on the same `LightGlueRuntime` instance
|
||||
When the second call enters
|
||||
Then `LightGlueConcurrentAccessError` is raised in the second thread; the first thread completes normally
|
||||
|
||||
**AC-5: Construction-time guard**
|
||||
Given `LightGlueRuntime(engine_handle=None)`
|
||||
When construction runs
|
||||
Then `LightGlueRuntimeError` is raised mentioning `engine_handle`
|
||||
|
||||
**AC-6: No upward imports — R14 structural fix**
|
||||
Given the helper module
|
||||
When a static-import check runs across `gps_denied_onboard.helpers.lightglue_runtime`
|
||||
Then it imports ONLY from `_types`, numpy, and stdlib — NO imports from `gps_denied_onboard.components.*` (verified by importlinter or grep gate in CI)
|
||||
|
||||
**AC-7: Determinism downstream of the engine**
|
||||
Given the same `(features_a, features_b)` pair matched twice with the same `engine_handle`
|
||||
When `match` runs both times
|
||||
Then both `CorrespondenceSet` outputs are byte-equal (engine determinism is a C7 concern; this AC asserts the helper itself adds no non-determinism)
|
||||
|
||||
## Non-Functional Requirements
|
||||
|
||||
**Performance**
|
||||
- `match` p99 ≤ 30 ms on Tier-2 with the production DISK+LightGlue engine on a typical K=10 candidate pair (matches the per-frame budget for C2.5's K=10 → N=3 funnel).
|
||||
- Helper-level overhead (excluding the engine call itself) ≤ 100 µs — verified via a benchmark that swaps in a stub engine handle.
|
||||
|
||||
**Reliability**
|
||||
- `LightGlueRuntimeError` and `LightGlueConcurrentAccessError` are the ONLY exception types the public surface raises. Engine-internal exceptions MUST be wrapped.
|
||||
- Pure-deterministic given a deterministic engine; the helper itself adds no random state.
|
||||
|
||||
**Concurrency**
|
||||
- Single-thread by contract. The concurrent-access guard is the runtime invariant detector — any composition-root regression that wires the runtime into multiple threads is caught immediately rather than producing GPU memory corruption.
|
||||
|
||||
## Unit Tests
|
||||
|
||||
| AC Ref | What to Test | Required Outcome |
|
||||
|--------|-------------|-----------------|
|
||||
| AC-1 | single-pair match on synthetic-overlap fixture | non-empty `CorrespondenceSet` |
|
||||
| AC-2 | batch of 3 pairs | three results in input order; per-pair invariants match AC-1 |
|
||||
| AC-3 | dim-mismatched features | `LightGlueRuntimeError`; message names expected & actual dims |
|
||||
| AC-4 | two threads call `match` simultaneously | one succeeds; the second raises `LightGlueConcurrentAccessError` |
|
||||
| AC-5 | construct with `engine_handle=None` | `LightGlueRuntimeError` |
|
||||
| AC-6 | importlinter / grep gate over `helpers/lightglue_runtime.py` | no `components.*` imports |
|
||||
| AC-7 | same pair matched twice | byte-equal outputs (with deterministic stub engine) |
|
||||
| NFR-perf | microbench `match` overhead with stub engine (10k iterations on Tier-2 fixture) | helper overhead ≤ 100 µs |
|
||||
|
||||
## Constraints
|
||||
|
||||
- Public surface frozen by `_docs/02_document/contracts/shared_helpers/lightglue_runtime.md` v1.0.0.
|
||||
- Layer 1 Foundation only. NO upward imports — this is the load-bearing constraint for the R14 fix.
|
||||
- The `EngineHandle` Protocol must be defined in `_types/manifests.py` (AZ-263 / E-BOOT) so this helper can reference it without importing C7. If `_types/manifests.py` does not yet define the Protocol surface (`forward(...)`, `descriptor_dim`), this task adds it — that is the only `_types` edit allowed by this task.
|
||||
- No new dependency beyond what AZ-263 / E-BOOT pinned.
|
||||
|
||||
## Risks & Mitigation
|
||||
|
||||
**Risk 1: Composition root accidentally creates two runtimes (one for C2.5, one for C3)**
|
||||
- *Risk*: Future composition-root refactor instantiates `LightGlueRuntime` twice; engine memory doubles, behaviour drifts.
|
||||
- *Mitigation*: The composition-root contract test (E-CC-CONF / AZ-246, AZ-269/AZ-270 in scope) already verifies cardinality of cross-cutting helpers. This task's contract documents that EXACTLY ONE instance is expected; the composition-root validator is the enforcement point.
|
||||
|
||||
**Risk 2: Concurrent-access guard introduces hot-path overhead**
|
||||
- *Risk*: A naive `threading.Lock` on every `match` call adds 100s of µs.
|
||||
- *Mitigation*: The guard uses a non-blocking `threading.local()`-style check or a `Lock(blocking=False).acquire()` pattern that simply RAISES on contention rather than serialising callers — the contract is "concurrent calls are a bug", not "serialise concurrent callers". NFR-perf microbench validates the overhead budget.
|
||||
|
||||
**Risk 3: A future backbone needs a different match shape**
|
||||
- *Risk*: A new feature backbone produces 5-tuple correspondences instead of the current 4-tuple (e.g., adds confidence per match).
|
||||
- *Mitigation*: The contract version bump path is documented (`Versioning Rules` section). Adding a field is non-breaking IF consumers tolerate the extra field; otherwise it is a major-version contract change with a deprecation pass.
|
||||
|
||||
## Runtime Completeness
|
||||
|
||||
- **Named capability**: shared LightGlue inference runtime with single-CUDA-stream guarantee + R14 structural cycle fix (architecture / E-CC-HELPERS / `03_helper_lightglue_runtime.md`).
|
||||
- **Production code that must exist**: real `EngineHandle`-backed match dispatch; real concurrent-access guard; real descriptor-dim validation.
|
||||
- **Allowed external stubs**: a deterministic stub `EngineHandle` is allowed in tests (and recommended for AC-7 determinism) but production paths use C7's real engine.
|
||||
- **Unacceptable substitutes**: bypassing the concurrent-access guard with `threading.Lock` (silently serialising callers); allowing each consumer to construct its own runtime; reintroducing a C2.5 → C3 (or C3 → C2.5) import to "share state". Any of those reintroduces R14.
|
||||
|
||||
## Contract
|
||||
|
||||
This task produces the contract at `_docs/02_document/contracts/shared_helpers/lightglue_runtime.md`.
|
||||
Consumers MUST read that file — not this task spec — to discover the interface.
|
||||
@@ -1,162 +0,0 @@
|
||||
# RansacFilter Helper Module
|
||||
|
||||
**Task**: AZ-282_ransac_filter
|
||||
**Name**: RansacFilter Helper
|
||||
**Description**: Implement the shared `RansacFilter` helper that wraps OpenCV's RANSAC inlier filtering and reprojection-residual computation. Used by C2.5 (single-pair LightGlue inlier counting), C3 (2D-2D RANSAC over cross-domain correspondences), C3.5 (residual recompute after AdHoP refinement), and C4 (per-frame final reprojection residual for FDR provenance). Stateless static-only design; deterministic by setting OpenCV's RNG seed.
|
||||
**Complexity**: 2 points
|
||||
**Dependencies**: AZ-263_initial_structure, AZ-277_se3_utils
|
||||
**Component**: shared.helpers.ransac_filter (cross-cutting; epic AZ-264 / E-CC-HELPERS)
|
||||
**Tracker**: AZ-282
|
||||
**Epic**: AZ-264 (E-CC-HELPERS)
|
||||
|
||||
### Document Dependencies
|
||||
|
||||
- `_docs/02_document/contracts/shared_helpers/ransac_filter.md` — frozen public interface this task produces.
|
||||
- `_docs/02_document/contracts/shared_helpers/se3_utils.md` — the `SE3` (re-exported GTSAM `Pose3`) type used in `compute_reprojection_residual`.
|
||||
- `_docs/02_document/common-helpers/07_helper_ransac_filter.md` — design rationale.
|
||||
|
||||
## Problem
|
||||
|
||||
Four components run RANSAC over 2D-2D correspondences (C2.5, C3, C3.5) or compute reprojection residuals after PnP (C4). Without a shared helper:
|
||||
- Each component grows its own `cv2.findHomography(..., cv2.RANSAC)` wrapper; some set the RNG seed, some don't, producing non-determinism in tests.
|
||||
- The "what counts as the residual" definition drifts — some compute mean, some median, some mean-of-squares — so the FDR `mre_px` field means different things in different components.
|
||||
- C4's "final per-frame residual" used by FDR provenance ends up with a slightly different formula than C3.5's "did refinement help?" residual; cross-component comparisons in post-flight analysis become apples-to-oranges.
|
||||
|
||||
## Outcome
|
||||
|
||||
- A single `helpers.ransac_filter` module is the only path through which any onboard process runs 2D-2D RANSAC or computes a reprojection residual.
|
||||
- RANSAC is deterministic given a fixed seed: `cv2.setRNGSeed(0)` (or the explicit `seed` kwarg where supported) is set inside `filter_correspondences` so the same input correspondences always produce the same `RansacResult`.
|
||||
- Residual statistic is fixed to the MEDIAN (in pixels), not mean — outliers in the 2D residual distribution do not bias the consumer's quality signal. Documented in the contract.
|
||||
- C4's `solvePnPRansac` continues to use OpenCV's internal RANSAC (out of contract); this helper covers the standalone 2D-2D case + the standalone reprojection-residual computation that lives outside the PnP call.
|
||||
|
||||
## Scope
|
||||
|
||||
### Included
|
||||
|
||||
- `RansacFilter` static methods: `filter_correspondences`, `compute_reprojection_residual`.
|
||||
- `RansacResult` frozen dataclass with `inlier_correspondences`, `inlier_count`, `outlier_count`, `median_residual_px`.
|
||||
- `RansacFilterError` exception type.
|
||||
- Public interface contract published at `_docs/02_document/contracts/shared_helpers/ransac_filter.md`.
|
||||
|
||||
### Excluded
|
||||
|
||||
- 2D-3D RANSAC inside `solvePnPRansac` — OpenCV does it internally; this helper does not wrap it.
|
||||
- Per-component RANSAC threshold defaults — they are documented in C2.5, C3, C3.5, C4 specs. This helper takes the threshold as a parameter.
|
||||
- Adaptive RANSAC (PROSAC, USAC) — out of scope for v1.0.0.
|
||||
- GPU-accelerated RANSAC — out of scope for v1.0.0.
|
||||
- Confidence / iteration-count tuning of `cv2.findHomography` — exposed only via `ransac_threshold_px` for v1.0.0.
|
||||
|
||||
## Acceptance Criteria
|
||||
|
||||
**AC-1: Clean correspondences yield 100 % inliers + zero residual**
|
||||
Given 100 correspondences generated from a known homography (no outliers, no noise)
|
||||
When `filter_correspondences` runs with `ransac_threshold_px=1.5`
|
||||
Then `RansacResult` has `inlier_count == 100`, `outlier_count == 0`, `median_residual_px ≈ 0.0` within `atol=1e-6`
|
||||
|
||||
**AC-2: Mixed correspondences produce expected inlier band**
|
||||
Given 80 inliers (perfect homography) + 20 outliers (random noise)
|
||||
When `filter_correspondences` runs with `ransac_threshold_px=1.5`
|
||||
Then `inlier_count` ∈ `[78, 82]` (RANSAC noise tolerance) AND `outlier_count == 100 - inlier_count`
|
||||
|
||||
**AC-3: Determinism — fixed seed**
|
||||
Given the same input correspondences
|
||||
When `filter_correspondences` runs twice
|
||||
Then both `RansacResult` outputs are byte-equal (numpy arrays equal-by-value, all fields match)
|
||||
|
||||
**AC-4: Reprojection residual ≈ 0 on clean inliers + known pose**
|
||||
Given 4+ correspondences generated from a known camera intrinsics + pose pair (no noise)
|
||||
When `compute_reprojection_residual` runs
|
||||
Then the result is ≈ 0.0 within `atol=1e-6`
|
||||
|
||||
**AC-5: Empty inlier array → NaN, no exception**
|
||||
Given an empty inlier array (shape `(0, 4)`)
|
||||
When `compute_reprojection_residual` runs
|
||||
Then it returns `NaN` (no exception)
|
||||
|
||||
**AC-6: Shape contract on correspondences**
|
||||
Given correspondences with shape `(N, 3)` (missing one coordinate column)
|
||||
When `filter_correspondences` runs
|
||||
Then `RansacFilterError` is raised mentioning the expected shape `(N, 4)`
|
||||
|
||||
**AC-7: Threshold guard**
|
||||
Given `ransac_threshold_px = -1.0`
|
||||
When `filter_correspondences` runs
|
||||
Then `RansacFilterError` is raised mentioning the positive-threshold requirement
|
||||
|
||||
**AC-8: Minimum point count**
|
||||
Given correspondences with shape `(3, 4)` (fewer than 4 points)
|
||||
When `filter_correspondences` runs
|
||||
Then `RansacFilterError` is raised mentioning the 4-point minimum for homography RANSAC
|
||||
|
||||
**AC-9: K shape contract in residual call**
|
||||
Given `K` of shape `(4, 4)`
|
||||
When `compute_reprojection_residual` runs
|
||||
Then `RansacFilterError` is raised mentioning the expected `(3, 3)` shape
|
||||
|
||||
**AC-10: No upward imports (Layer 1 invariant)**
|
||||
Given the helper module
|
||||
When a static-import check runs
|
||||
Then it imports ONLY from `_types`, `helpers.se3_utils`, `cv2`, `numpy`, and stdlib — no `gps_denied_onboard.components.*` imports anywhere
|
||||
|
||||
## Non-Functional Requirements
|
||||
|
||||
**Performance**
|
||||
- `filter_correspondences` p99 ≤ 5 ms on Tier-2 for `N=200` correspondences (matches the C3 cross-domain matcher's per-candidate budget).
|
||||
- `compute_reprojection_residual` p99 ≤ 1 ms on Tier-2 for `I=100` inliers.
|
||||
- Helper-level overhead vs. inline OpenCV ≤ 5 % (per E-CC-HELPERS hot-path NFR).
|
||||
|
||||
**Reliability**
|
||||
- `RansacFilterError` is the ONLY exception type the public surface raises on shape / dtype / threshold violations. OpenCV's lower-level exceptions MUST be wrapped.
|
||||
- Determinism: same input → byte-equal `RansacResult` outputs (RNG seed is set inside the helper).
|
||||
|
||||
## Unit Tests
|
||||
|
||||
| AC Ref | What to Test | Required Outcome |
|
||||
|--------|-------------|-----------------|
|
||||
| AC-1 | 100 clean correspondences, threshold 1.5 px | `inlier_count == 100`; `median_residual_px ≈ 0.0` |
|
||||
| AC-2 | 80 inliers + 20 outliers | `inlier_count ∈ [78, 82]` |
|
||||
| AC-3 | same input run twice | byte-equal `RansacResult` outputs |
|
||||
| AC-4 | clean inliers + known pose | residual ≈ 0.0 |
|
||||
| AC-5 | empty inlier array | returns `NaN`; no exception |
|
||||
| AC-6 | shape `(N, 3)` | `RansacFilterError`; mentions `(N, 4)` |
|
||||
| AC-7 | `ransac_threshold_px = -1.0` | `RansacFilterError`; mentions positive threshold |
|
||||
| AC-8 | shape `(3, 4)` | `RansacFilterError`; mentions 4-point minimum |
|
||||
| AC-9 | `K.shape = (4, 4)` | `RansacFilterError`; mentions `(3, 3)` |
|
||||
| AC-10 | importlinter / grep gate | no `components.*` imports |
|
||||
| NFR-perf | microbench `filter_correspondences` (10k iters on Tier-2 fixture, N=200) | p99 ≤ 5 ms |
|
||||
| NFR-perf-residual | microbench `compute_reprojection_residual` (10k iters on Tier-2 fixture, I=100) | p99 ≤ 1 ms |
|
||||
|
||||
## Constraints
|
||||
|
||||
- Public surface frozen by `_docs/02_document/contracts/shared_helpers/ransac_filter.md` v1.0.0.
|
||||
- Layer 1 Foundation only.
|
||||
- `cv2` is the single RANSAC backend; pinned in `pyproject.toml` at AZ-263 / E-BOOT (OpenCV ≥ 4.12.0 per D-CROSS-CVE-1).
|
||||
- Static-only design satisfies `coderule.mdc`.
|
||||
- Helper depends on `helpers.se3_utils` for the `SE3` type alias (allowed — same Layer 1).
|
||||
- No new dependency beyond what AZ-263 / E-BOOT pinned.
|
||||
|
||||
## Risks & Mitigation
|
||||
|
||||
**Risk 1: Determinism regression on OpenCV upgrade**
|
||||
- *Risk*: A future OpenCV version changes the RNG seeding API; tests start failing intermittently because the fixed-seed contract no longer holds.
|
||||
- *Mitigation*: AC-3 is the canary. The helper's own test suite catches the regression on dependency upgrade before consumers do. The contract test should also pin a known-good `RansacResult` fixture for a reference input so any drift surfaces immediately.
|
||||
|
||||
**Risk 2: Median vs mean residual confusion**
|
||||
- *Risk*: A future contributor "improves" the helper to return mean residual; consumers' quality thresholds (which were tuned against median) become looser; FDR data is inconsistent across cycles.
|
||||
- *Mitigation*: The contract `Invariants` section pins median. AC-1's `≈ 0.0` clean residual is the same for both, but AC-2's mixed-quality residual would shift on a mean→median change; the test fixture pins enough digits that the regression is caught.
|
||||
|
||||
**Risk 3: 4-point minimum too low for noisy inputs**
|
||||
- *Risk*: With exactly 4 points, RANSAC has zero redundancy; one bad point makes the homography unstable.
|
||||
- *Mitigation*: The 4-point minimum is the OpenCV minimum — consumers that need higher minimums set them via `min_inliers` and check `RansacResult.inlier_count` themselves. The contract documents that `min_inliers` is informational; the consumer is the gatekeeper.
|
||||
|
||||
## Runtime Completeness
|
||||
|
||||
- **Named capability**: 2D-2D RANSAC inlier filtering + median reprojection residual via OpenCV (architecture / E-CC-HELPERS / `07_helper_ransac_filter.md`).
|
||||
- **Production code that must exist**: real `cv2.findHomography(..., cv2.RANSAC)`-backed filtering; real `cv2.projectPoints`-backed residual computation; real fixed-seed determinism wiring.
|
||||
- **Allowed external stubs**: none — OpenCV is the production runtime.
|
||||
- **Unacceptable substitutes**: hand-rolled RANSAC (numerical instability, no OpenCV-tested edge-case handling); skipping RNG seed (silent intermittent test failures); switching residual statistic from median to mean (changes consumer-visible quality signal).
|
||||
|
||||
## Contract
|
||||
|
||||
This task produces the contract at `_docs/02_document/contracts/shared_helpers/ransac_filter.md`.
|
||||
Consumers MUST read that file — not this task spec — to discover the interface.
|
||||
Reference in New Issue
Block a user