Add polarization calibration and DoF analysis improvements#4
Add polarization calibration and DoF analysis improvements#4ChrisCai1007 wants to merge 1 commit into
Conversation
ChrisCai1007
changed the base branch from
main
to
codex/usaf-channel-manual-points
|
This PR is stacked on top of the USAF manual-points PR. For review, could you please review both PRs together as one change set? Review order:
I changed this PR's base branch to Related PRs:
|
JonathanZhuMD
left a comment
JonathanZhuMD
left a comment
There was a problem hiding this comment.
Review — Reject (P0 found)
Thanks for the breadth here — three initiatives, new tests, all 15 CI checks green. Unfortunately there's a real P0 in the polarization pipeline that the existing test surface doesn't catch. Plus a few P1s worth addressing before re-requesting review.
P0 — Play-mode virtual polarization channels render as zeros / 500
Symptom. Selecting S0, DoLP, or AoP (or the HG-/LG- prefixed variants) in Play mode shows:
- a solid-black thumbnail and zero-everywhere stats/histogram for frame 0, OR
- a 500
KeyErrorfromframe_channel_thumbnail/frame_channel_stats/frame_channel_histogram/frame_channel_pixel/ MP4 + tiled export for any frame N > 0 in an H5 source.
Root cause. polarization.append_virtual_channels (mantisanalysis/polarization.py, new file) inserts np.zeros(shape, dtype=np.float32) placeholders for the virtual channels into LoadedSource.channels. The real values are only computed in server._channel_image via _pol.compute_virtual_channel, which the analyze endpoints route through. But every per-frame endpoint reads chs = src.extract_frame(idx); chs[channel] directly — and LoadedSource.extract_frame (mantisanalysis/session.py:239-306) only synthesizes HDR channels for RGB_NIR / legacy_gsbsi_rgb_nir modes. It never calls a polarization helper. Yet LoadedSource.channel_keys exposes S0/DoLP/AoP to _summary_dict, so the frontend offers them as selectable channels.
Where to fix. Mirror the HDR pattern: add a _resolve_polarization_channels(src, chs) helper that, for polarization modes, replaces the placeholder keys in the per-frame chs dict with _pol.compute_virtual_channel(...) output (honoring dark + calibration state on the source). Call it from extract_frame directly, or at every chs = src.extract_frame(...) callsite in server.py (~10 callsites: 1893, 2116, 2281, 2372, 2543, 2653, 2767, 2957, 3539, 4502, 5179 in current main).
Test gap. Add a Play-mode test asserting that frame_channel_thumbnail / frame_channel_stats on a polarization source returns non-zero values for S0/DoLP/AoP when calibration is enabled (and a sensible non-NaN range when it isn't).
P1 — set_polarization_calibration_enabled reads source state outside the lock
mantisanalysis/session.py in the new set_polarization_calibration_enabled method: is_polarization_mode(src.isp_mode_id), src.polarization_calibration, src.has_dark, and _validate_polcal_profile_for_source(src, ...) all execute before with self._lock:. Window: thread A's set_..._enabled(True) validates while has_dark=True, thread B's clear_dark lands and sets polarization_calibration_enabled=False inside the lock, then thread A flips it back to True inside its own lock. Result: calibration enabled with no dark frame; _channel_image then gates apply_dark on src.has_dark so the calibration runs against raw analyzers (silently wrong polarization output).
Fix: take self._lock once for the full validate-and-set sequence.
P1 — compute_all_metrics=True payload + CPU
The new all-metrics branch in dof_analyze (mantisanalysis/server.py) runs analyze_dof_multi four times with bootstrap=True (default n_boot=200), build_heatmap=True, fit_tilt_plane=True, then serializes all four full per-channel trees plus _attach_dof_metric_sweeps. For 10 channels × 4 lines × 4 metrics × ~1024-sample lines the response JSON can easily run multi-MB and the wall time is ~4× a single-metric run. Worth measuring on a representative recording before close; consider stripping duplicate focus arrays from the sweep overlays or gating heatmap/bootstrap behind an explicit opt-in for the all-metrics path.
P1 — Stacked-PR / rebase ordering
This PR's base is codex/usaf-channel-manual-points (PR #3), which has just been merged into main with a doc-conflict resolution. Please rebase this branch onto current main before pushing the fixes — the diff currently sits on top of an old PR #3 HEAD, and a clean rebase + python -m pytest tests/unit/test_usaf_manual_points_api.py tests/unit/test_polarization_calibration.py -q + scripts/smoke_test.py --tier 0/1/2/3 post-rebase is the smallest re-verification I'd ask for.
P2 — minor
compute_profile_contrast_bandinmantisanalysis/dof_analysis.pyuses a Pythonfor i in range(n): np.percentile(...)sliding window — ~4 s wasted per all-metrics analyze on representative inputs. Considerscipy.ndimage.percentile_filteror a rolling sort. Math is otherwise correct (divide-by-zero guarded, NaN-masked, out-of-bounds clipped).attach_polarization_calibration_from_bytesusestempfile.NamedTemporaryFile(delete=False)+try/finally + unlink. Same SIGKILL leak as the existing dark-attach pattern; not a regression.- Scope discipline: three initiatives in one PR makes the change very hard to review and selectively revert. For the resubmit, consider splitting polarization vs. DoF profiles vs. DoF threshold bands into separate PRs that can land independently.
What I verified passes
- No changes to
mantisanalysis/extract.py— GSense Bayer constants untouched ✓ - Channel-key schema is extended (new virtual S0/DoLP/AoP keys), not renamed — explicitly documented in this PR's DECISIONS.md entry ✓
mantisanalysis/polarization.pyis pure NumPy + h5py (no Qt, no FastAPI, no React) ✓- Stokes math (
stokes_from_analyzers,compute_aop_dolp) is correct: S0 = I0+I90, S1 = I0−I90, S2 = I45−I135; AoP = ½·atan2(S2,S1) mod 180°; DoLP clipped to [0,1]; NaN propagation guarded ✓ - 4 new FastAPI endpoints follow existing error patterns (KeyError→404, ValueError→422) ✓
- Polarization calibration enable is gated on mode + dark + profile + shape (modulo the P1 race above) ✓
clear_darkcorrectly disablespolarization_calibration_enabled✓- Transient frame creation propagates polcal state ✓
The Stokes/DoLP/AoP analysis math itself looks solid. The breakage is in the plumbing between LoadedSource.channels placeholders and the per-frame access path. Once that's wired (likely a small helper in extract_frame), I'd expect a clean re-review.
🤖 Generated with Claude Code
2 participants
Summary
This PR adds the next set of MantisAnalysis improvements after the USAF manual-points PR.
Major additions
S0,DoLP, andAoPchannels..polcal.h5calibration loading and an Apply map workflow gated by polarization mode, dark frame, and calibration profile.laplacian,brenner,tenengrad, andfft_hfresult trees.Fixes / behavior improvements
Verification
npm run buildPASSnpm run lintPASS with existing warnings onlypython scripts/smoke_test.py --tier 0PASSpython scripts/smoke_test.py --tier 1PASSpython scripts/smoke_test.py --tier 2PASSpython scripts/smoke_test.py --tier 3PASSpython -m pytest -qPASS, 315 passed / 4 skippedpytest -m web_smoke -qskipped because Playwright is not installed locally