Fix 3D multi-robot safety metric, threshold display, and animation output path - #369
Merged
Conversation
_compute_distance_metrics reshaped each robot's state block to its full width and took the norm across every column, so the "minimum inter-robot distance" safety metric reported sqrt(|dp|^2 + |dv|^2) rather than |dp|. The overstatement grows with closing speed, making the plot least trustworthy exactly when robots converge: on a captured trajectory it read 11.94 where the true separation was 0.65, and two robots sharing a position while closing at 3 m/s each are reported 6.0 apart against a 0.5 threshold. goal_dists and obs_dists in the same function already sliced positions correctly; this makes the third metric agree with them. The new test covers the coincident-but-moving case, velocity independence, and an independent min-over-pairs oracle, plus non-regression pins for the sdim==3 layout and the two metrics that were already correct.
Threshold reference line: - The manim backend drew no threshold line, so its inter-robot panel could not be read as satisfied vs violated. Plumbs `threshold` through visualize_3d_multi_robot -> render_multi_robot_3d -> MultiRobot3DScene -> _build_chart_panel. - All three backends took the panel's y-limit from the data alone, so a run violating the constraint throughout clipped the reference line off-screen and rendered as a clean plot. The limit now includes the threshold and guards non-finite data (all-NaN, or the all-inf that a single robot produces). - Adds `safety_radius` so the drawn per-robot bubbles derive from the same constant the controller enforces instead of matching by coincidence. - Drops two warnings claiming the manim backend ignores the distance panels: it forwards the data and renders them, so the warnings stated the opposite of what happened. Animation output path: render_multi_robot_3d handed manim a full filename (manim appends its own extension) and never set the format, so requesting "anim.gif" produced "anim.gif.mp4" under <media_dir>/videos/<quality>/ and left whatever already existed at the requested path. Regenerating a showcase asset therefore appeared to succeed while the advertised file stayed stale. The renderer now honors the requested extension and copies the finished file to that path. Tests: the manim dispatch tests patched cbfkit.utils.animator's re-export of _HAS_MANIM, which _require_manim() never consults, so they passed only because manim was absent and would fail wherever it is installed. They now patch the authoritative binding and no longer skip, since nothing under test needs manim itself. Tutorial: seeds the scenario, wires D_MIN_SQUARED into the barrier constraint that previously hard-coded it (enforced value unchanged at 0.25, so the dynamics are identical), draws three independent initial velocity components instead of broadcasting one scalar, passes the enforced constant as both threshold and bubble radius, and prints the path the backend actually returns.
… metric The previous asset predates the inter-robot distance fix, so its "Min Inter-Robot Dist" panel plotted velocity-inflated separation and carried no threshold reference line. Regenerated from the now-seeded tutorial, so the panel shows true separation against a visible threshold. Encoded with a 128-colour palette and bayer dithering: visually equivalent to the default palette at this size, since the frame is largely flat background and solid colours, and 28% smaller.
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
Three fixes to the 3D multi-robot visualization. The controller and the enforced constraints are untouched; this only changes what gets measured and drawn.
The distance metric was wrong
_compute_distance_metricsreshaped each robot's state block to its full width (6 columns here: x, y, z, vx, vy, vz) and took the norm across all of them, so the "min inter-robot distance" panel plottedsqrt(|dp|^2 + |dv|^2)instead of|dp|. The error grows with closing speed, which made the plot least accurate at the one moment it matters. On a captured trajectory it read 11.94 where the true separation was 0.65. Two robots sitting at the same point and closing at 3 m/s each get reported as 6.0 apart against a threshold of 0.5.goal_distsandobs_distsin the same function already sliced positions correctly. This makes the third metric agree with them.The threshold line was missing or clipped
The manim backend drew no threshold line at all, so its panel could not be read as satisfied or violated. All three backends also took the y limit from the data alone, which pushed the line off screen in the case you most need it: a run that violates the constraint throughout rendered as a clean plot. The limit now accounts for the threshold and handles non-finite data.
There is also a new
safety_radiusargument, so the per-robot bubbles are sized from the same constant the controller enforces rather than happening to match it.Animations were written to the wrong path
render_multi_robot_3dhanded manim a full filename, which manim appends its own extension to, and never set the format. Asking foranim.gifproducedanim.gif.mp4under<media_dir>/videos/<quality>/and left whatever already sat at the requested path alone. Regenerating a showcase asset looked like it worked while the advertised file stayed months out of date. The renderer now honors the extension you ask for and copies the finished file there.Two warnings are gone too. The manim path warned that
include_min_distance_plotwould be ignored, then forwarded the data and drew the panel anyway.Checks
Removing the safety filter puts all four robots through the obstacle (closest approach 0.63 against a required 8.0), while the filtered run stays at 10.83. The CBF was doing its job the whole time; only the reporting was off. On the same simulation the reported metric went from 18.33x over-stated to exact.
The new
tests/test_utils/test_visualization_helpers_3d.pycovers the coincident-but-moving case, velocity independence, and an independent min-over-pairs check, plus pins for thesdim == 3layout and the two metrics that were already right. Three of its six tests fail against the old code; the other three guard against breaking things in the other direction.The manim dispatch tests were patching
cbfkit.utils.animator's re-export of_HAS_MANIM, which_require_manim()never consults. They passed only because manim was absent and would have failed on any machine that had it. They now patch the real binding, and no longer skip, since nothing they test needs manim itself.Full fast suite: 490 passed, 4 skipped.
ruff check srcand black are clean.Tutorial
It now seeds the scenario, which previously drew new random initial conditions on every run.
D_MIN_SQUAREDis wired into the barrier constraint that used to hard-code the value; the enforced number stays 0.25, so the dynamics are identical. Initial velocities are drawn as three independent components instead of one scalar broadcast across all three. The tutorial passes the enforced constant as both threshold and bubble radius, and prints the path the backend actually returns.The showcase animation is re-rendered from this tutorial, so its panel now shows true separation against a visible threshold.