Skip to content

Add optional rerun debug view of placement validation - #984

Draft
xyao-nv wants to merge 12 commits into
mainfrom
xyao/feature/placement-rerun-debug-view
Draft

Add optional rerun debug view of placement validation#984
xyao-nv wants to merge 12 commits into
mainfrom
xyao/feature/placement-rerun-debug-view

Conversation

@xyao-nv

@xyao-nv xyao-nv commented Jul 31, 2026

Copy link
Copy Markdown
Collaborator

Summary

Rerun debug view of placement validation

Detailed description

  • Adds an opt-in Rerun view that draws every candidate layout using bbox
  • When in curobo image and reachability validation is enabled, the cuRobo reachability check draws the robot base, the grasps it solved, reachable/unreachable per target, and IK error.
  • Turned on from config, not a CLI flag: placement_validators.debug_visualize (and/ordebug_visualize_rrd_path for headless runs) in an env graph YAML, or ObjectPlacerParams(debug_visualize=True).
  • Off by default and off in every shipped env. Include instructions on how to enable it.

Example viz

In base docker (during solver validation):
https://github.com/user-attachments/assets/d73882ee-ab13-4309-aa7b-f19de10ad13c

In curobo docker (where reachability is enabled)
https://github.com/user-attachments/assets/55d99e37-6eff-4558-8444-e0a54f251f04

@xyao-nv xyao-nv changed the title Xyao/feature/placement rerun debug view Add optional rerun debug view of placement validation Jul 31, 2026

@alexmillane alexmillane left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thanks for adding this functionality! Looks pretty good.

See suggestions below!

Comment thread isaaclab_arena/environment_spec/arena_env_graph_conversion_utils.py Outdated
Comment thread isaaclab_arena/environment_spec/arena_env_graph_types.py Outdated
self.params = params or ObjectPlacerParams()
self._solver = RelationSolver(params=self.params.solver_params)
# Populated before the validators are built so a check can add its own layer to the same view.
self.params.debug_visualizer = get_or_create_placement_visualizer(self.params)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Suggestion that we only keep params in the params struct? I.e. no objects that implement run-time functionality.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Make sense, moved it to placer's member.

# Per-check candidate indices that check was actually run on; expensive checks skip candidates.
evaluated_slots_by_check: dict[str, list[int]] = {}
layout_pass_verdicts_by_check: dict[str, list[bool]] = {}
# Layouts are drawn before the checks run so a check's own layer lands on top of its candidate.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

"so a check's own layer lands on top of its candidate."

Not sure I understand that. What's a check's "layer"?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Agent's docstring is confusing af. Updated with mine.

Comment thread isaaclab_arena/relations/object_placer.py Outdated
debug_visualize_rrd_path: str | None = None
"""Path to record the debug visualization to as a Rerun ``.rrd`` file, for headless runs.

Enables the visualization on its own; combine with ``debug_visualize`` to both record and watch live."""

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Enables the visualization on its own? Does this mean that this also starts the live view?

Consider rewording.

Comment thread isaaclab_arena/relations/placement_visualizer.py
Comment thread isaaclab_arena_curobo/ik_reachability_validator.py Outdated
self,
positions: dict[ObjectBase, tuple[float, float, float]],
orientations: dict[ObjectBase, float],
batch_slot: int,

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

What is a batch_slot.

This is diffcult to detemine what it is from the variable name.

I would guess that slot is coming from rerun? This is just the index of the solution among the batch right? Suggestion to use batch_idx, solution_idx.

The slot terminology says what this variable is used for inside the function, not what it actually is. In general, prefer describing what something is, rather than what you intend to use it for.

Comment thread isaaclab_arena_curobo/reachability_visualizer.py
xyao-nv added 10 commits August 2, 2026 22:49
Placement solving is sim-free, so its candidate layouts have been hard to
inspect without starting Isaac Sim. Opting in via ObjectPlacerParams now
streams every candidate to a Rerun viewer (or an .rrd, for headless runs).

- Add PlacementRerunVisualizer: one process-wide view that draws each
  candidate's object boxes and the checks that accepted or rejected it,
  one frame per candidate on a `candidate` timeline.
- Turn it on with ObjectPlacerParams.debug_visualize /
  debug_visualize_rrd_path; ObjectPlacer builds the view before its
  validators so a check can add its own layer to it.
- Let the cuRobo reachability check layer on what only it knows: the
  robot base frame, the top-down grasps it solved, per-target
  reachable/unreachable verdicts, and its IK error scalars.
- Keep frames aligned across checks: expensive checks only see the
  candidates that passed the cheap ones, so the placer tells the view
  which candidates the running check was handed.

Signed-off-by: Xinjie Yao <xyao@nvidia.com>
- Add debug_visualize / debug_visualize_rrd_path to PlacementValidatorSpec so
  an env's YAML can ask for the Rerun view without editing Python.
- Forward both through build_checks_for_placer_params into ObjectPlacerParams.
- Turn the view on in the butter_raisin_box scene as a worked example.
- Cover the default-off and forwarding paths in the graph-spec tests.

Signed-off-by: Xinjie Yao <xyao@nvidia.com>
- Spawn the Rerun viewer under `setpriv --pdeathsig TERM` instead of `rr.spawn()`, so the
  kernel closes the window when the run exits.
- Isaac Sim's `SimulationApp.close()` ends in `os._exit()`, so no `atexit` hook can do this.
- Stops a stale viewer from holding port 9876, which silently made the next run's spawn a no-op.
- Wait for the viewer to serve before logging, replacing the readiness wait `rr.spawn()` did.

Signed-off-by: Xinjie Yao <xyao@nvidia.com>
- Take `debug_visualize: true` out of the shipped butter/raisin scene, where it spawned a viewer
  window on every build, and move the worked example to a test-data graph YAML.
- Guard that with a test asserting no versioned env under `isaaclab_arena_environments` asks for it.
- Track which candidates each check was run on, so the view no longer draws an expensive check as
  rejecting a layout it skipped; the summary count now derives from the same record.
- Warn when the viewer port is already served, and confirm the spawned viewer is still alive once
  it answers, so a run cannot silently log into somebody else's window.
- Fall back to killing a viewer that ignores SIGTERM instead of raising out of close().

Signed-off-by: Xinjie Yao <xyao@nvidia.com>
- Acceptance now follows the placer, which gates layouts on required_checks alone.
- A failure the placer does not gate on reads as accepted, naming the advisory check.
- Pull the wording out into summarize_candidate_verdict() so the rule is unit-testable.

Signed-off-by: Xinjie Yao <xyao@nvidia.com>
- Show the env graph YAML and Python routes in the visualizer module, pointing at the worked example.
- Say the same from the reachability check, whose layer only appears once that view is on.
- Drop ACCEPTED_COLOR / REJECTED_COLOR, which nothing has ever drawn with.

Signed-off-by: Xinjie Yao <xyao@nvidia.com>
Keep ObjectPlacerParams data-only: checks reach the live Rerun view
through get_active_placement_visualizer() rather than a field on the
params, and the per-batch layout/verdict logging moves off ObjectPlacer
onto the visualizer that owns it.

Rename debug_visualize_rrd_path to debug_visualize_output_path and the
opaque batch "slot" to batch_index, shorten the docstrings that
described code living elsewhere, and fix the enabled check name in the
worked YAML example (ik_reachability -> ik_reachable), which was being
silently dropped as unregistered.

Signed-off-by: Xinjie Yao <xyao@nvidia.com>
Say what each piece does in plain terms: shorter constant docs, one
straightforward line per function, and the remaining rationale (setpriv,
the shared view, the deepcopy guard) stated directly instead of alluded
to.

Signed-off-by: Xinjie Yao <xyao@nvidia.com>
@xyao-nv
xyao-nv force-pushed the xyao/feature/placement-rerun-debug-view branch from 64f02fe to 5c0022c Compare August 3, 2026 05:59
def close(self) -> None:
"""Flush pending data and shut down the viewer window this run spawned. Idempotent.

Only needed to close the window early -- a run that just exits leaves it to the viewer's

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

rm

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants