Skip to content

fix: vectorize LineSegments3D encoding and decoding - #3956

Merged
leshy merged 3 commits into
mainfrom
andrew/fix/vectorize-line-segments
Sep 8, 2026
Merged

fix: vectorize LineSegments3D encoding and decoding#3956
leshy merged 3 commits into
mainfrom
andrew/fix/vectorize-line-segments

Conversation

@aclauer

@aclauer aclauer commented Sep 5, 2026

Copy link
Copy Markdown
Collaborator

Contribution path

  • Small, safe change that does not need a tracking issue
  • Linked issue or discussion: DIM-XXX / #XXX / URL

Problem

Visualizing large sets of line segments (like large maps in the planner) would take a long time (over 500ms in some cases I was testing).

Solution

Rewrite the decoding and to_rerun functions to be vectorized instead of iterating.

segments original new
1000 3.86 ms 0.019 ms
5000 20.21 ms 0.038 ms
20000 110.49 ms 0.201 ms
60000 404.24 ms 0.493 ms

How to Test

AI assistance

Checklist

  • I have read and approved the CLA.

@codecov

codecov Bot commented Sep 6, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 80.55556% with 14 lines in your changes missing coverage. Please review.
✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
dimos/msgs/nav_msgs/LineSegments3D.py 67.50% 12 Missing and 1 partial ⚠️
dimos/navigation/nav_3d/mls_planner/viz.py 0.00% 1 Missing ⚠️
@@            Coverage Diff             @@
##             main    #3956      +/-   ##
==========================================
+ Coverage   77.80%   77.82%   +0.02%     
==========================================
  Files        1392     1393       +1     
  Lines      131250   131277      +27     
  Branches    11390    11388       -2     
==========================================
+ Hits       102115   102171      +56     
+ Misses      25887    25857      -30     
- Partials     3248     3249       +1     
Components Coverage Δ
Tests 94.74% <100.00%> (+<0.01%) ⬆️
Flag Coverage Δ
OS-ubuntu-24.04-arm 74.16% <80.55%> (+0.02%) ⬆️
OS-ubuntu-latest 74.67% <80.55%> (+0.02%) ⬆️
Py-3.10 74.67% <80.55%> (+0.03%) ⬆️
Py-3.11 74.67% <80.55%> (+0.02%) ⬆️
Py-3.12 74.67% <80.55%> (+0.02%) ⬆️
Py-3.13 74.67% <80.55%> (+0.02%) ⬆️
Py-3.14 74.67% <80.55%> (+0.02%) ⬆️
Py-3.14t 74.67% <80.55%> (+0.02%) ⬆️
SelfHosted-Large 30.76% <33.33%> (+<0.01%) ⬆️
SelfHosted-Linux 35.78% <33.33%> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
dimos/msgs/nav_msgs/test_LineSegments3D.py 100.00% <100.00%> (ø)
dimos/navigation/nav_3d/mls_planner/viz.py 47.22% <0.00%> (+12.52%) ⬆️
dimos/msgs/nav_msgs/LineSegments3D.py 72.88% <67.50%> (+36.88%) ⬆️
🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@greptile-apps

greptile-apps Bot commented Sep 6, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

The vectorized line-segment visualization accepts a weight array whose length differs from the number of segments. That can either crash Rerun conversion or hand it incomplete color metadata.

Confidence Score: 4/5

Safe to merge with a non-blocking follow-up to reject malformed segment-weight inputs.

One confirmed non-blocking input-validation issue remains. Direct execution reproduced both the empty-weight crash and inconsistent Rerun metadata for a short weight array.

Files Needing Attention: dimos/msgs/nav_msgs/LineSegments3D.py

T-Rex T-Rex Logs

What T-Rex did

  • T-Rex produced a proof for a posted P2 finding and linked it to the corresponding review comment.
  • A Python artifact containing the P2 finding proof was prepared for review.
  • A log artifact for the P2 finding run was uploaded and is accessible via URL.
  • T-Rex authored and ran the line-segments-weights-validation.py script to validate the contract and captured the output showing an exception traceback and a LineStrips3D result.
  • The validation run produced a log artifact with a URL pointing to the execution results.

View all artifacts

T-Rex Ran code and verified through T-Rex

Comments Outside Diff (1)

  1. General comment

    P2 LineSegments3D permits mismatched weight and segment counts

    • Bug
      • The constructor accepts an empty or otherwise mismatched weight array for nonempty segments. Empty weights cause to_rerun() to raise during log_w.min()/max(). A one-weight/two-segment input returns an rr.LineStrips3D whose runtime representation has two strips but only one color and one radius, producing an inconsistent rendering handoff.
    • Cause
      • Lines 64-68 reshape supplied weights without checking len(weights) == len(segments). Lines 118-124 then assume the weights can both be reduced and paired one-per-strip.
    • Fix
      • Validate in __init__ that a supplied weight array has exactly one item per segment, raising a clear ValueError otherwise. Optionally retain a defensive cardinality check in to_rerun() for objects mutated after construction.

    T-Rex Ran code and verified through T-Rex

Reviews (1): Last reviewed commit: "Just keep the fast" | Re-trigger Greptile

Comment thread dimos/msgs/nav_msgs/LineSegments3D.py
@github-actions github-actions Bot added the ready-to-merge Required CI checks have passed on this PR label Sep 6, 2026
@leshy
leshy added this pull request to the merge queue Sep 8, 2026
Merged via the queue into main with commit 1f64a7f Sep 8, 2026
36 checks passed
@leshy
leshy deleted the andrew/fix/vectorize-line-segments branch September 8, 2026 23:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready-to-merge Required CI checks have passed on this PR

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants