Add Live Plot Windows in all Visualizers#6533
Conversation
matthewtrepte
requested review from
Mayankm96,
jtigue-bdai,
ooctipus and
pascal-roth
as code owners
Greptile SummaryThis PR adds live-plot windows to the Newton, Rerun, and Viser visualizer backends by introducing a new backend-agnostic
Confidence Score: 3/5Two correctness issues need addressing before merge: the Rerun config default change affects all existing users' memory footprint, and the Kit path silently drops caller-specified term filters. The keep_scalar_history default flip in RerunVisualizerCfg from False to True quietly changes runtime behavior for every Rerun-based training run. Separately, KitVisualizer.add_live_plots accepts a term_names filter but never passes it into the ManagerLiveVisualizerCfg constructors. The Newton, Viser, and base-class additions are clean. source/isaaclab_visualizers/isaaclab_visualizers/rerun/rerun_visualizer_cfg.py and source/isaaclab_visualizers/isaaclab_visualizers/kit/kit_visualizer.py Important Files Changed
Reviews (1): Last reviewed commit: "impl live plot for each visualizer" | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| keep_scalar_history: bool = True | ||
| """Keep scalar/plot history in timeline. | ||
|
|
||
| Must be ``True`` for live plots to accumulate as a time-series; ``False`` logs each value as | ||
| static (no timeline association), which overwrites the previous value and produces a flat chart. | ||
| """ |
There was a problem hiding this comment.
keep_scalar_history was previously False — explicitly chosen to keep memory constant for training workloads (as the old docstring noted). Flipping the default to True means every existing Rerun-based training run will now accumulate all scalar history in memory for the full duration, even when live plots are not in use. Over long runs this can cause OOM. The change to True is only needed when live plots are active; consider leaving the default False and programmatically setting it to True inside RerunVisualizer.add_live_plots() when live-plot sources are registered, so the memory trade-off is opt-in rather than global.
| self.kit_manager_visualizers: dict[str, ManagerLiveVisualizer] = { | ||
| name: ManagerLiveVisualizer( | ||
| manager=mgr, | ||
| cfg=ManagerLiveVisualizerCfg(manager_name=name), | ||
| ) | ||
| for name, mgr in managers.items() | ||
| } |
There was a problem hiding this comment.
term_names filter silently dropped for Kit widget path
The term_names: dict[str, list[str]] | None parameter is accepted by add_live_plots but never forwarded when constructing ManagerLiveVisualizerCfg. Any caller that passes a per-manager term allowlist to limit what gets plotted will find the Kit omni.ui widget ignores it and always shows all terms. The fix is to pass term_names=(term_names or {}).get(name) as ManagerLiveVisualizerCfg(manager_name=name, term_names=(term_names or {}).get(name)).
| def collect(self, env_idx: int) -> dict[str, list[float]]: | ||
| """Pull current term values from the manager for one environment. | ||
|
|
||
| Only terms whose values are scalar or low-dimensional vectors (up to | ||
| :data:`_MAX_SCALAR_COMPONENTS` elements) are returned. Image-shaped data (3-D+) is | ||
| omitted and should be handled separately with :meth:`collect_images`. | ||
|
|
||
| Args: | ||
| env_idx: Environment index to sample. | ||
|
|
||
| Returns: | ||
| Mapping from term name to a list of float values sampled this step. | ||
| """ | ||
| result: dict[str, list[float]] = {} | ||
| try: | ||
| for term_name, values in self._manager.get_active_iterable_terms(env_idx=env_idx): | ||
| if self._allowed is not None and term_name not in self._allowed: | ||
| continue | ||
| arr = np.asarray(values, dtype=np.float32) | ||
| if arr.ndim > 2: | ||
| continue | ||
| flat = arr.flatten() | ||
| if flat.size > _MAX_SCALAR_COMPONENTS: | ||
| continue | ||
| result[term_name] = [float(v) for v in flat] | ||
| except Exception: | ||
| logger.debug( | ||
| "[ManagerLivePlots] Failed to collect terms from %s.", self.manager_name, exc_info=True | ||
| ) | ||
| return result | ||
|
|
||
| def collect_images(self, env_idx: int) -> dict[str, np.ndarray]: | ||
| """Pull image-shaped term values (3-D+) from the manager. | ||
|
|
||
| Args: | ||
| env_idx: Environment index to sample. | ||
|
|
||
| Returns: | ||
| Mapping from term name to a NumPy array with shape ``(H, W[, C])``. | ||
| """ | ||
| result: dict[str, np.ndarray] = {} | ||
| try: | ||
| for term_name, values in self._manager.get_active_iterable_terms(env_idx=env_idx): | ||
| if self._allowed is not None and term_name not in self._allowed: | ||
| continue | ||
| arr = np.asarray(values) | ||
| if arr.ndim >= 3: | ||
| result[term_name] = arr | ||
| except Exception: | ||
| logger.debug( | ||
| "[ManagerLivePlots] Failed to collect images from %s.", self.manager_name, exc_info=True | ||
| ) | ||
| return result |
There was a problem hiding this comment.
get_active_iterable_terms iterated twice per step when both scalars and images are needed
collect() and collect_images() each call self._manager.get_active_iterable_terms(env_idx=env_idx) independently. In _debug_vis_callback both are called back-to-back, so every manager's terms are traversed twice per update tick. For managers with many terms this doubles the CPU/GPU tensor-copy work. A single private helper that partitions results into scalars vs. images in one pass would eliminate the redundant iteration.
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!
- Fix ruff-format failures in manager_live_plots.py and rerun_visualizer.py - Add changelog skip fragment for isaaclab_experimental (supporting changes only, feature entries live in isaaclab and isaaclab_visualizers) - Revert keep_scalar_history default to False to avoid unbounded memory growth during training; set it to True inside add_live_plots() when live-plot sources are registered so the trade-off is opt-in - Forward term_names per-manager allowlist to ManagerLiveVisualizerCfg in KitVisualizer.add_live_plots() so Kit widget respects term filters
- Change enable_live_plots default from True to False in VisualizerCfg to avoid overhead during headless training runs - Add Live Plots section to visualization.rst covering all four backends, manager-based env requirement, and per-backend behavior - Remove stale limitation entry claiming live plots are Kit-only - Remove stale Viser note claiming live plots are unsupported
github-actions
Bot
added
the
documentation
1 participant
Description
Add Live Plot windows to Rerun, Viser, and Rerun. Also modify Kit Live Plot feature to make a more general impl for all visualizers.
Type of change
Screenshots
Please attach before and after screenshots of the change if applicable.
Checklist
pre-commitchecks with./isaaclab.sh --formatconfig/extension.tomlfileCONTRIBUTORS.mdor my name already exists there