[Newton] Adding visualizer support for Newton #4099
+2,074
−433
Conversation
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
Signed-off-by: matthewtrepte <mtrepte@nvidia.com>
Signed-off-by: matthewtrepte <mtrepte@nvidia.com>
…s default cfgs if no visualizers are specified
Contributor
Greptile OverviewGreptile SummaryThis PR introduces a comprehensive visualizer system for Isaac Lab 3.0, replacing the single Important Files Changed
Confidence score: 3/5
Sequence DiagramsequenceDiagram
participant User
participant AppLauncher
participant SimulationContext
participant SceneDataProvider
participant NewtonManager
participant NewtonVisualizer
participant NewtonViewerGL
participant TrainingLoop
User->>AppLauncher: "python train.py --visualizer newton"
AppLauncher->>AppLauncher: "parse --visualizer flag"
AppLauncher->>AppLauncher: "store visualizer type in carb settings"
AppLauncher->>SimulationContext: "create simulation context"
SimulationContext->>SimulationContext: "read visualizer setting from carb"
alt visualizer requested
SimulationContext->>SimulationContext: "_create_default_visualizer_configs()"
SimulationContext->>SceneDataProvider: "create provider with visualizer configs"
SceneDataProvider->>SceneDataProvider: "determine active backends"
end
SimulationContext->>NewtonManager: "start_simulation()"
NewtonManager->>NewtonManager: "initialize physics model and state"
SimulationContext->>SimulationContext: "reset()"
SimulationContext->>SimulationContext: "initialize_visualizers()"
alt newton visualizer enabled
SimulationContext->>NewtonVisualizer: "create_visualizer() from config"
SimulationContext->>NewtonVisualizer: "initialize(scene_data)"
NewtonVisualizer->>SceneDataProvider: "get_model() and get_state()"
SceneDataProvider->>NewtonManager: "access physics data"
NewtonManager-->>SceneDataProvider: "return model and state"
SceneDataProvider-->>NewtonVisualizer: "return physics data"
NewtonVisualizer->>NewtonViewerGL: "create viewer with metadata"
NewtonViewerGL->>NewtonViewerGL: "set_model() and configure camera"
NewtonViewerGL-->>NewtonVisualizer: "viewer initialized"
NewtonVisualizer-->>SimulationContext: "visualizer ready"
end
loop training/simulation
TrainingLoop->>SimulationContext: "step(render=True)"
SimulationContext->>NewtonManager: "step()"
NewtonManager->>NewtonManager: "simulate physics"
SimulationContext->>SceneDataProvider: "update()"
SceneDataProvider->>NewtonManager: "sync fabric transforms for OV"
SimulationContext->>SimulationContext: "step_visualizers(dt)"
SimulationContext->>NewtonVisualizer: "step(dt, state=None)"
NewtonVisualizer->>SceneDataProvider: "get updated state"
SceneDataProvider->>NewtonManager: "access latest physics state"
NewtonManager-->>SceneDataProvider: "return current state"
SceneDataProvider-->>NewtonVisualizer: "return updated state"
alt not paused and update frequency met
NewtonVisualizer->>NewtonViewerGL: "begin_frame(sim_time)"
NewtonVisualizer->>NewtonViewerGL: "log_state(state)"
NewtonVisualizer->>NewtonViewerGL: "end_frame()"
NewtonViewerGL->>NewtonViewerGL: "render 3D scene"
end
end
User->>TrainingLoop: "stop training"
TrainingLoop->>SimulationContext: "close_visualizers()"
SimulationContext->>NewtonVisualizer: "close()"
NewtonVisualizer->>NewtonViewerGL: "cleanup resources"
SimulationContext->>SimulationContext: "clear_instance()"
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| visualizer_train_mode: bool = True | ||
| """Whether the visualizer is in training mode (True) or play mode (False).""" | ||
|
|
||
| solver_cfg: NewtonSolverCfg = MJWarpSolverCfg() |
Contributor
There was a problem hiding this comment.
style: Missing docstring for the solver_cfg attribute
Suggested change
| solver_cfg: NewtonSolverCfg = MJWarpSolverCfg() | |
| solver_cfg: NewtonSolverCfg = MJWarpSolverCfg() | |
| """Configuration for the Newton solver.""" |
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!
Comment on lines
+19
to
+21
| This module uses a registry pattern to decouple visualizer instantiation from specific types. | ||
| Visualizer implementations can register themselves using the `register_visualizer` decorator, | ||
| and configs can create visualizers via the `create_visualizer()` factory method. |
Contributor
There was a problem hiding this comment.
syntax: Documentation mentions register_visualizer decorator and create_visualizer() factory method that don't exist in the implementation
Suggested change
| This module uses a registry pattern to decouple visualizer instantiation from specific types. | |
| Visualizer implementations can register themselves using the `register_visualizer` decorator, | |
| and configs can create visualizers via the `create_visualizer()` factory method. | |
| This module uses a registry pattern to decouple visualizer instantiation from specific types. | |
| Visualizer implementations are lazy-loaded through the `get_visualizer_class()` factory function. |
Comment on lines
+174
to
+176
| # Calculate camera direction vector (from position to target) | ||
| cam_pos = self.cfg.camera_position | ||
| cam_target = self.cfg.camera_target |
Contributor
There was a problem hiding this comment.
logic: camera position and target are retrieved but not used in the blueprint configuration. Should these camera parameters be applied to the blueprint's Spatial3DView configuration?
| for visualizer in self.env.sim._visualizers: | ||
| # Check if visualizer supports live plots and has it enabled | ||
| if hasattr(visualizer, "cfg") and hasattr(visualizer.cfg, "enable_live_plots"): | ||
| if visualizer.supports_live_plots() and visualizer.cfg.enable_live_plots: |
Contributor
There was a problem hiding this comment.
logic: This method call could raise an AttributeError if the visualizer object doesn't implement supports_live_plots(). Consider adding a hasattr check.
Comment on lines
+748
to
+751
| # Handle training pause - block until resumed | ||
| while visualizer.is_training_paused() and visualizer.is_running(): | ||
| # Visualizers fetch backend-specific state themselves | ||
| visualizer.step(0.0, state=None) |
Contributor
There was a problem hiding this comment.
logic: This blocking while loop could cause the main thread to hang if a visualizer never resumes from pause. Consider adding a timeout or making this non-blocking with a sleep.
7 tasks
Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com> Signed-off-by: Kelly Guo <kellyg@nvidia.com>
Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com> Signed-off-by: Kelly Guo <kellyg@nvidia.com>
Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com> Signed-off-by: Kelly Guo <kellyg@nvidia.com>
Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com> Signed-off-by: Kelly Guo <kellyg@nvidia.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
Description
Building on top of #3979 from @matthewtrepte
Adds additional
--visualizerflag for specifying which visualizer(s) to enable.Updates numpy to >=2 to support new rerun package that resolves a flashing screen issue.
Introduces default visualizer configs in the simulation config so that users can launch any visualizer without explicitly adding new configs into the environments.
Type of change
Checklist
pre-commitchecks with./isaaclab.sh --formatconfig/extension.tomlfileCONTRIBUTORS.mdor my name already exists there