dimos map tooling, stream alignment#2306
Conversation
Replaces the monolithic pgo_then_voxels with four primitives over mem2 Streams and Transforms: pgo_keyframes(lidar) -> Stream[Keyframe] keyframes_to_corrections(kfs) -> Stream[Transform] (world_corrected <- world_raw) make_interpolator(corrections) -> (ts) -> Transform (SLERP + linear, endpoint-clipped) apply_corrections(stream, corr) -> Stream[T] (shuffles obs.pose) Drift correction is now a first-class, reusable Stream[Transform] that any pose-stamped consumer can apply — same math as before (rigid T_corr = T_global @ T_local^-1 per keyframe, SLERP + linear interpolation between bracketing keyframes), just composable. dimos map --pgo migrates to the new primitives; pgo_then_voxels is deleted. The internal _SimplePGO / PGOConfig / _KeyPose machinery stays in pgo.py and is imported by pgo2.
Renames: pgo.py -> pgo_internals.py (gtsam/ICP machinery) pgo2.py -> pgo.py (public Stream-shaped API) `dimos.mapping.relocalization.pgo` is now the canonical import path (`pgo_keyframes`, `keyframes_to_corrections`, `make_interpolator`, `apply_corrections`, `correction_at`, `Keyframe`). The internal _SimplePGO / PGOConfig / _KeyPose / _icp / _voxel_downsample helpers live in pgo_internals.py and are imported lazily inside pgo.py to keep gtsam off the public-API import path.
Adds test_pgo.py covering pose normalization on Observation, the Transform↔Pose3 conversion helpers, the interpolator edge cases, apply_corrections behavior, and a real-recording smoke test (skipped when data/go2_short.db is absent). Annotates the two map.py helpers so they pass mypy. Removes leftover section divider comments to satisfy the no-section-markers project rule.
The colors-copy block in `PointCloud2.transform` calls `self.pointcloud` to check `has_colors()`, which forces a tensor->legacy conversion on every invocation. With `pgo --full-pgo` rebuilding from hundreds of lidar frames, that hidden allocation dominates the per-frame cost. The colors path was lidar-irrelevant (lidar clouds have no colors anyway) and the feature it was meant to support never landed; remove it so transform() stays a clean numpy round-trip.
…s into feat/ivan/pgo_rewrite
Group the map CLI scripts (globalmap, pose_fill, rename, replay, replay_marker, summary) plus their assets and dataset_validation doc under a cli/ subpackage. distance.py stays in utils/ as a shared lib. The map CLI test moves alongside as cli/test_cli.py.
These files were inadvertently diverged on this branch. Restore both to main; the map-marker CLI callers remain compatible with main's DetectMarkers API.
These end-to-end CLI tests forked python -m dimos.robot.cli.dimos per case, re-paying the heavy per-verb imports (rerun, open3d, voxel) 7 times. Run them in-process via Typer's CliRunner so those imports are paid once: 33.7s -> 7.5s, all 7 still pass. Trade-off: shared interpreter, no per-test process isolation.
Add from_time/to_time (relative to the first observation) and from_timestamp/to_timestamp (absolute epoch seconds) for windowing a stream by time. A trailing to_time is a duration measured from the current start, so from_time(2).to_time(30) reads as "skip 2s, take the following 30s"; frames mix freely (from_timestamp(ts).to_time(30)). Shared base for the stream-alignment (#2306) and go2dds (#2314) branches, which both need this windowing API.
…arch
- pgo_keyframes: widen stream param to Iterable[Observation[PointCloud2]]
(clears the mypy gate; it never uses Stream methods)
- pose_fill_db: order_by("ts") on both align inputs — align requires
ts-ascending iteration, but sqlite defaults to id ASC (insertion order),
which would silently mis-pair out-of-order recordings
- add CI-running unit tests for Observation.with_pose (lazy payload kept)
and pose_fill(mount=...) composition
- remove the unreferenced autoresearch/ research drop (near-duplicate of
pgo_auto.py + scaffolding); pgo_auto.py kept
Add from_time/to_time (relative to the first observation) and from_timestamp/to_timestamp (absolute epoch seconds) for windowing a stream by time. A trailing to_time is a duration measured from the current start, so from_time(2).to_time(30) reads as "skip 2s, take the following 30s"; frames mix freely (from_timestamp(ts).to_time(30)). Shared base for the stream-alignment (#2306) and go2dds (#2314) branches, which both need this windowing API.
Pulls the stairs dataset LFS pointer from feat/ivan/go2stairs so the map tooling can use it; object already on remote LFS store.
…nment # Conflicts: # dimos/memory2/stream.py # dimos/memory2/test_stream.py
The self-hosted CI image's published :dev tag is stale (predates libturbojpeg0-dev in docker/python/Dockerfile), so TurboJPEG() raises at runtime and the four verbs that decode the color_image stream fail. Guard them with skipif so they skip on a lib-less image and still run where the native lib is present.
| if no_gui: | ||
| print(f"open with: rerun {out}") | ||
| else: | ||
| subprocess.Popen(["rerun", str(out)]) |
There was a problem hiding this comment.
You maintain to reference to this? How is it ever closed?
| --out mid360_renamed.db \\ | ||
| --rename go2_lidar=lidar \\ | ||
| --rename lidar=fastlio_lidar \\ | ||
| --rename odometry=fastlio_odometry |
There was a problem hiding this comment.
This is meant to be used through dimos map rename, no? Then please don't document running the file itself.
|
|
||
| for p in primary_iter: | ||
| # Advance the cursor until `nxt` sits past `p`; `prev` trails just behind. | ||
| while nxt is not None and nxt.ts <= p.ts: |
There was a problem hiding this comment.
Why <=? If nxt.ts == p.ts isn't that the best possible solution?
| pose=pose, | ||
| _data=_UNLOADED, | ||
| _loader=lambda: self.data, | ||
| _data_lock=threading.Lock(), |
There was a problem hiding this comment.
| _data_lock=threading.Lock(), |
Already the default. (But it's a bit odd to pass locks around. Usually synchronization should be owned by the object protecting the state, not having an external call telling it what to synchronize on.)
| """Print ``Store.summary()`` for a memory2 sqlite recording. | ||
|
|
||
| Usage: | ||
| uv run python -m dimos.mapping.utils.cli.summary mid360 |
There was a problem hiding this comment.
| uv run python -m dimos.mapping.utils.cli.summary mid360 | |
| uv run dimos map summary mid360 |
| # Skip placeholder poses (origin position OR zero quaternion). | ||
| if pose[0] == 0 and pose[1] == 0 and pose[2] == 0: | ||
| continue | ||
| if pose[3] == 0 and pose[4] == 0 and pose[5] == 0 and (pose[6] == 0 or pose[6] == 1): | ||
| continue |
There was a problem hiding this comment.
Valid values shouldn't be placeholder values. I don't see why valid values like this should be skipped? Can't you use None (an invalid pose) as a placeholder?
dimos maputilities for dataset collection teamyou can ignore those, they are temporary,
changing pose source for observation stream, rendering global map, loop closures etc. lots of tooling (temporary and will change a lot)
dataset validation instructions document
stream alignment
a very simple stream.align tool - this needs much more work. multi stream input and output, interpolation of poses/transforms etc