Skip to content

recording based false-negative eval for the MLS planner - #3154

Open
GuraseesWasNotAvailable wants to merge 3 commits into
dimensionalOS:mainfrom
GuraseesWasNotAvailable:feat/mls-planner-recording-eval
Open

recording based false-negative eval for the MLS planner#3154
GuraseesWasNotAvailable wants to merge 3 commits into
dimensionalOS:mainfrom
GuraseesWasNotAvailable:feat/mls-planner-recording-eval

Conversation

@GuraseesWasNotAvailable

Copy link
Copy Markdown

Contribution path

Problem

The MLS planner can return "no path" when a path actually exists. There was no automated way to (a) score the planner on a real recording or (b) tell whether a change to the mapper improves planning.

Solution

A recording-based false-negative eval. The robot traverses the whole recording in one continuous run so any points on its odometry trajectory are provably reachable. The eval replays a recording through the online pipeline, then asks plan(A, B) for trajectory-sampled start/goal pairs and plan() -> None on a walkable pair is a false negative.

Baseline on mid360_athens_stairs: 112/132 false negatives across 3 auto-detected floors. false negatives rise with separation.

Ships as recording_eval.py + a self_hosted regression test (test_recording_eval.py).

How to Test

uv run python -m dimos.navigation.nav_3d.mls_planner.recording_eval data/mid360_athens_stairs.db
uv run python -m dimos.navigation.nav_3d.mls_planner.recording_eval data/mid360_athens_stairs.db --sweep
uv run pytest dimos/navigation/nav_3d/mls_planner/test_recording_eval.py -m self_hosted

AI assistance

Claude Code (Opus 4.8) was used for the following:

  1. Understanding the codebase.
  2. Syntax and eval test
  3. Running the analysis in a WSL build environment.

Checklist

  • I have read and approved the CLA.

GuraseesWasNotAvailable and others added 2 commits July 22, 2026 17:03
Adds a self_hosted eval (issue dimensionalOS#2996) that replays a recorded lidar+odom dataset through the production pipeline (RayTraceMap -> MLSPlanner.update_region) and uses the robot's own traversed trajectory as ground truth to measure planner false negatives across a floor x floor matrix. Includes a union-find disconnect diagnostic that confirms each false negative is a genuine 'separate connected surface components' failure, and a coarse box-vs-voxel safety guardrail (reported, not gated). test_recording_eval.py wraps it as a self_hosted regression baseline.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Addresses maintainer feedback (the ray-tracing voxel mapper is the highest-leverage module; 'planning is only as good as the map'):

- Auto-detect floors from the trajectory height histogram (detect_floors) so the eval runs on any recording without hardcoded strata.

- Make the ray-tracing MapperConfig and PlannerConfig first-class inputs, and add sweep() to A/B several mapper configs against planning accuracy -- the loop for measuring whether a mapper change reduces false negatives. Supports capped replays (max_frames) for fast iteration.

- Add a false-negative-vs-separation breakdown to the scorecard; tune the safety-guardrail radius to the planner's wall_clearance.

Baseline on athens_stairs: 112/132 false negatives (84.8%) across 3 detected floors, every one a confirmed graph disconnect.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@greptile-apps

greptile-apps Bot commented Jul 23, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

Adds a recording-based MLS planner evaluation:

  • Replays aligned lidar and odometry through the ray-tracing mapper and MLS planner.
  • Measures false negatives, graph connectivity, distance/floor breakdowns, and approximate path safety.
  • Adds a self-hosted regression suite with a fixed 132-pair baseline denominator and meaningful diagnostic checks.

Confidence Score: 5/5

The PR appears safe to merge.

No blocking failures remain within the scope of the previous review threads.

Important Files Changed

Filename Overview
dimos/navigation/nav_3d/mls_planner/recording_eval.py Adds the recording replay, trajectory sampling, planner evaluation, connectivity diagnostics, safety checks, reporting, and sweep CLI.
dimos/navigation/nav_3d/mls_planner/test_recording_eval.py Adds self-hosted regression coverage and adequately addresses the previously reported empty-replay, denominator, and tautological-assertion concerns.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart LR
    DB[Recording database] --> ALIGN[Align lidar and odometry]
    ALIGN --> MAP[RayTraceMap]
    MAP --> MLS[MLSPlanner map and graph]
    ALIGN --> TRAJ[Foot trajectory]
    TRAJ --> FLOORS[Detect floors and sample pairs]
    FLOORS --> PLAN[Plan each start/goal pair]
    MLS --> PLAN
    PLAN --> SCORE[False-negative and safety scorecard]
Loading

Reviews (2): Last reviewed commit: "fix(nav): address review — empty-replay ..." | Re-trigger Greptile

The robot lingers on floors (tall histogram bins) and passes through stairs
quickly (sparse bins), so the well-populated modes are the floors. Returns a
sorted list of z levels (pose height)."""
z = feet[:, 2] + robot_height

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Empty replay crashes floor detection

When the required streams contain no lidar/odometry pairs within align_tol, build_map returns an empty one-dimensional feet array and this indexing raises IndexError, causing both the normal CLI and sweep to terminate instead of reporting that no aligned observations were available.


def test_false_negatives_do_not_regress(scorecard):
"""The headline metric: feasible pairs the planner fails to route."""
assert scorecard.false_neg <= BASELINE_FALSE_NEG, (

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Baseline ignores evaluation denominator

When the override or canonical recording yields only two detected floors, the evaluation tests at most 56 pairs but still compares against the 112-failure baseline measured over 132 pairs, so even a 100% false-negative rate passes this regression gate.

Comment on lines +101 to +103
assert n_disc <= n_fn, f"cell {i}->{j}: {n_disc} disconnects > {n_fn} false negatives"


Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Assertions are true by construction

n_disc <= n_fn is guaranteed because disconnects are counted only after the same iteration increments n_fn; similarly, unsafe >= 0 only checks an increment-only counter initialized to zero. These tests remain green when disconnect classification or safety checking is never meaningfully exercised, creating misleading regression coverage.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

…n gate

Greptile review fixes:

- build_map/detect_floors now raise a clear ValueError on an empty (no aligned lidar/odom) replay instead of an opaque IndexError.

- Pin the regression denominator: assert total == EXPECTED_TOTAL (132) before the absolute false-negative baseline, so fewer detected floors can't pass with a worse rate.

- Replace two vacuous assertions: require >=90% of false negatives to be confirmed disconnects, and assert paths were actually produced (unsafe <= produced). Add a unit test for the empty-trajectory guard.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@TomCC7 TomCC7 added the first-time-contributor PR opened by an author who had not previously committed to this repository label Jul 24, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

first-time-contributor PR opened by an author who had not previously committed to this repository

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants