-
Notifications
You must be signed in to change notification settings - Fork 2.8k
[Newton] Lots of hackery to get kit-optional paths working with and without OV #4123
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Signed-off-by: matthewtrepte <[email protected]>
Signed-off-by: matthewtrepte <[email protected]>
…s default cfgs if no visualizers are specified
Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com> Signed-off-by: Kelly Guo <[email protected]>
Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com> Signed-off-by: Kelly Guo <[email protected]>
Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com> Signed-off-by: Kelly Guo <[email protected]>
Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com> Signed-off-by: Kelly Guo <[email protected]>
…ublic into decouple_app
Greptile OverviewGreptile SummaryThis PR introduces significant architectural changes to decouple Isaac Lab from Omniverse/Kit dependencies, enabling standalone operation with Newton and Rerun visualizers. Key Changes:
Known Issues (per PR description):
Confidence Score: 2/5
Important Files ChangedFile Analysis
Sequence DiagramsequenceDiagram
participant User
participant AppLauncher
participant SettingsManager
participant SimulationApp
participant SimulationContext
participant NewtonManager
participant Cloner
User->>AppLauncher: Initialize with args
AppLauncher->>AppLauncher: _check_if_omniverse_required()
alt Omniverse Required
AppLauncher->>SimulationApp: Create SimulationApp
AppLauncher->>SettingsManager: initialize_carb_settings()
SettingsManager->>SettingsManager: Use carb.settings
else Standalone Mode
AppLauncher->>SettingsManager: _store_settings_standalone()
SettingsManager->>SettingsManager: Use Python dict
end
User->>SimulationContext: Create SimulationContext(cfg)
SimulationContext->>SettingsManager: Get settings
SimulationContext->>SimulationContext: Create stage in memory
SimulationContext->>NewtonManager: Configure solver
User->>Cloner: clone_environments()
Cloner->>Cloner: newton_replicate()
Cloner->>NewtonManager: set_builder()
Cloner->>Cloner: usd_replicate()
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Additional Comments (1)
-
source/isaaclab/isaaclab/app/app_launcher.py, line 227-245 (link)syntax: Duplicate
visualizerproperty definition. The property is defined twice (lines 227-235 and 237-245). The second definition shadows the first.
43 files reviewed, 3 comments
| # from isaacsim.core.cloner import Cloner | ||
| # from isaacsim.core.version import get_version | ||
| # from omni.usd.commands import DeletePrimsCommand, MovePrimCommand | ||
| from pxr import Sdf, Usd, UsdGeom, UsdPhysics, UsdShade |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
syntax: Missing Gf import. The new create_default_xform_ops function (line 1752) uses Gf.Vec3d, Gf.Vec3f, Gf.Rotation, Gf.Quatd, Gf.Quatf, and Gf.Matrix4d but Gf is not imported.
| from pxr import Sdf, Usd, UsdGeom, UsdPhysics, UsdShade | |
| from pxr import Gf, Sdf, Usd, UsdGeom, UsdPhysics, UsdShade |
| else: | ||
| # otherwise, environment origins will be initialized during cloning at the end of environment creation | ||
| self._default_env_origins = None | ||
| self._default_env_origins, self.env_orientations = grid_transforms(self.cfg.num_envs, self.cfg.env_spacing, up_axis="z", device="cuda") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
logic: Hardcoded device="cuda" will cause failures on CPU-only systems. Should use self.device to respect the configured device.
| self._default_env_origins, self.env_orientations = grid_transforms(self.cfg.num_envs, self.cfg.env_spacing, up_axis="z", device="cuda") | |
| self._default_env_origins, self.env_orientations = grid_transforms(self.cfg.num_envs, self.cfg.env_spacing, up_axis="z", device=self.device) |
commit 43ea96c Author: Octi Zhang <[email protected]> Date: Wed Dec 3 16:26:01 2025 -0800 add shebang commit 5d8925e Author: Octi Zhang <[email protected]> Date: Wed Dec 3 16:23:49 2025 -0800 address comments commit 099a4b3 Author: Octi Zhang <[email protected]> Date: Wed Dec 3 16:10:36 2025 -0800 pass precommit commit c595cc4 Author: Octi Zhang <[email protected]> Date: Wed Dec 3 16:03:51 2025 -0800 support install omni-client as separate module
471bcf8 to
7aec53f
Compare
|
Closing, newer PR available at #4165 |
Description
A lot of hacking to decouple from OV - so far only tested on Cartpole.
Changes that need to be considered:
Known Issues: