feat(import): offline M3 backscatter store layer (#80)#82
Merged
Conversation
Thread intensity/beam_angle into GeoSounding, add nodeRecords() on GeoGrid, add importTiles() to MbesBackscatterStore, add geoGridToBackscatterTile/ mapSheetToBackscatterTiles in store_import, wire --bs-store flag in import_bag
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Extend the offline import_bag tool to also produce an MBES backscatter store layer (Processed) from the same CUBE pass that writes the bathy store. It surfaces the existing co-estimated, UNCORRECTED intensity (Node::extractNodeRecord); the beam-angle/GeoCoder correction stays deferred to cube#81 and node.cpp is untouched. Opt-in via --bs-store. - GeoGrid::nodeRecords(): enriched per-cell output (depth + co-estimated intensity/intensity_var) parallel to values(). - store_import: geoGridToBackscatterCells / mapSheetToBackscatterCells convert finite-intensity cells to MbesCell, keyed by CellIndex. - import_bag_main: thread intensity/beam_angle onto the projected soundings; --bs-store writes each cell via the store's public set() into the Processed layer and save()s with a provenance registry (timestamp + source_index per cell; sensor_class mbes-backscatter). - Tests: BackscatterCellsMatchGridRecords + BackscatterNaNPropagation. Layer = Processed (operator decision): the offline CUBE re-run is the authoritative off-boat product; cube#81 updates corrected values in place when it lands. The bathy layer stays Draft; the same-pass split is intentional. Uses the backscatter store's existing public set() API rather than adding a bulk importTiles() to the separate marine_mbes_backscatter_store repo, keeping the whole change within cube_bathymetry (one PR). Part of #80
A value-taking flag given as the last argument (e.g. `import_bag ... --bs-store` with no directory after it) advanced the arg cursor past end() and dereferenced it -- undefined behavior. Pre-existing pattern across every value flag (-o/-d/-r/--odom-topic/... and the new --bs-store); pre-push review-code flagged it "harden all or none". Add a bounds-checked next_value() helper used by every value flag, and mark usage() [[noreturn]] so the missing-value error path is statically known not to fall through to the deref. A trailing value flag now prints "error: option 'X' requires a value" + usage and exits, instead of crashing (smoke-tested: --bs-store, -r). Also resync the plan.md line-number reference (508-509). Part of #80
added 3 commits
June 27, 2026 20:33
cube#80 adds a real `<depend>marine_mbes_backscatter_store</depend>`, but the CI workflow pruned that package from the upstream workspace via COLCON_IGNORE (scope-only, from when nothing in cube depended on it). rosdep then failed on PR #82: "Cannot locate rosdep definition for [marine_mbes_backscatter_store]". Remove it from the AFTER_SETUP_UPSTREAM_WORKSPACE prune loop so it builds in upstream_ws alongside marine_bathymetry_store. It needs no skip-key (no marine_nav dep); its one extra source dep, marine_backscatter, is already built (not pruned) and adds no new system rosdep beyond what the prebaked deps image already carries for the bathy store. Comment updated. Part of #80
…review) Address the two optional suggestions from the Integrated Review on PR #82 (no must-fix): - CMakeLists: ament_export_dependencies now re-exports marine_bathymetry_store and marine_mbes_backscatter_store. The installed public header store_import.h includes both stores' headers, so a downstream find_package(cube_bathymetry) resolves them transitively without its own find_package. The bathy-store gap was pre-existing; the mbes one is new with #80. - test_store_import: BackscatterCellsMatchGridRecords now cross-checks the backscatter cell set against the bathy conversion's finite-depth cell set -- an independent production path (values()/geoGridToTile vs nodeRecords()), so a one-sided GGGS-walk/alignment bug would make the sets diverge and fail. 353 tests green. Part of #80
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
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.
Summary
Extends the offline
import_bagtool to also produce an MBES backscatter storelayer (Processed) from the same CUBE pass that already writes the bathy store —
the durable, offline counterpart to the live producer #78, and the ADR-0007 M3
backscatter producer the stack lacked. Opt-in via a new
--bs-store <dir>flag.What it does
GeoGrid::nodeRecords()— enriched per-cell output (depth + co-estimatedintensity/intensity_var, ADR-0007 D5), parallel tovalues().store_import:geoGridToBackscatterCells/mapSheetToBackscatterCells—convert finite-intensity cells to
MbesCell, keyed bygggs::CellIndex.import_bag_main— threadsintensity/beam_angleonto the projectedsoundings so CUBE co-estimates backscatter;
--bs-storewrites each cell viathe store's public
set()into the Processed layer andsave()s with aprovenance registry (per-cell
timestamp+source_index;sensor_class = mbes-backscatter).BackscatterCellsMatchGridRecords+BackscatterNaNPropagation(NaN-propagation guard so a future intensity-threading regression fails loudly).
marine_mbes_backscatter_store) added; plan kept in sync.Key decisions
is dropped). Per the Live SonarVisualizationTile producer + backscatter (intensity+beam_angle) wiring #78 plan-review + guidance comment on Offline M3 import: also produce a backscatter store layer (ADR-0007 M3 producer) #80:
node.cppemits the identity value behind a
TODO(#54-B/#15), andstore_importruns thesame estimator, so any
rx_anglecorrection belongs there once, sign-gated— tracked as M3 backscatter correction: empirical angular-response (ARA) normalization in the shared CUBE estimator #81.
node.cppis untouched; when M3 backscatter correction: empirical angular-response (ARA) normalization in the shared CUBE estimator #81 lands, both this offlinelayer and the Live SonarVisualizationTile producer + backscatter (intensity+beam_angle) wiring #78 live tile get corrected backscatter automatically.
authoritative off-boat product; M3 backscatter correction: empirical angular-response (ARA) normalization in the shared CUBE estimator #81 updates corrected values in place. The bathy
layer stays Draft — the same-pass split is intentional.
set()rather than adding a bulkimportTiles()to the separatemarine_mbes_backscatter_storerepo, keepingthe entire change within
cube_bathymetry(one PR).Testing
colcon build+colcon testgreen: 353 tests, 0 failures (2 new).--bs-store,-r, …)now prints
error: option 'X' requires a value+ usage and exits, instead ofthe prior
vector::end()deref (UB).Out of scope (pre-existing-pattern suggestions from local review)
ament_export_dependenciesre-exports onlyrclcppthoughstore_import.hnow includes the store headers — a pre-existing gap shared with
marine_bathymetry_store; left for a focused follow-up.BackscatterCellsMatchGridRecordsre-derives expected cells with the same walkas production (mitigated by absolute intensity/timestamp/source assertions).
Closes #80
Authored-By:
Claude Code AgentModel:
Claude Opus 4.8 (1M context)