Adds environment tests for Newton#4862
Conversation
Signed-off-by: Kelly Guo <kellyg@nvidia.com>
Greptile SummaryThis PR adds Newton physics backend support for environment testing by introducing Key changes:
Confidence Score: 4/5
Important Files Changed
Sequence DiagramsequenceDiagram
participant Test as test_environments_newton.py
participant Utils as env_test_utils.py
participant ParseCfg as parse_cfg.py
participant Env as ManagerBased/DirectRLEnv
participant SimCtx as SimulationContext
participant PhysMgr as PhysicsManager (Newton/PhysX)
Test->>Utils: _run_environments(task, device, num_envs, physics_preset_name="newton")
Utils->>Utils: _check_random_actions(...)
Utils->>ParseCfg: parse_env_cfg(task_name) → resolved env_cfg (default preset)
Utils->>ParseCfg: load_cfg_from_registry(task_name) → raw_cfg (PresetCfg wrappers intact)
Utils->>ParseCfg: apply_named_preset(env_cfg, raw_cfg, "newton")
Note over ParseCfg: Walks raw_cfg fields, replaces<br/>preset wrappers in env_cfg with<br/>the named preset value
Utils->>Env: gym.make(task_name, env_cfg=env_cfg)
Env->>SimCtx: SimulationContext(cfg.sim)
SimCtx->>PhysMgr: initialize Newton backend
Env->>Env: step(random_actions) × N
Note over Env: Gravity randomization calls<br/>env.sim.set_gravity(g)
Env->>SimCtx: set_gravity(gravity)
SimCtx->>PhysMgr: set_gravity(gravity)
PhysMgr-->>SimCtx: gravity applied
Env-->>Utils: obs, reward, done, info
Utils->>Env: env.close()
Last reviewed commit: ed34c01 |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
|
|
||
| # Note: Semantic Versioning is used: https://semver.org/ | ||
| version = "0.5.1" | ||
| version = "0.5.2" |
There was a problem hiding this comment.
Version skips 0.4.1 in extension.toml
The extension.toml jumps from 0.4.0 directly to 0.4.2. The CHANGELOG already contains a 0.4.1 (2026-03-03) entry (added in a prior PR), but its corresponding extension.toml bump was never committed. As a result the package version is permanently out of sync with its own changelog history. Consider whether 0.4.1 should have been released separately (and the toml fixed retroactively) or if the two changelog entries should be collapsed under 0.4.2.
| version = "0.5.2" | |
| version = "0.4.2" |
(The line content is already correct for this PR's intent; the concern is the missing intermediate 0.4.1 bump that should have been applied in the previous PR.)
| # device index. PhysxManager does the same via /physics/cudaDevice setting. | ||
| device = PhysicsManager._device | ||
| if "cuda" in device and ":" not in device: | ||
| cuda_device = sim_context.get_setting("/physics/cudaDevice") |
There was a problem hiding this comment.
int() cast may raise ValueError on float-valued setting
sim_context.get_setting("/physics/cudaDevice") can return a float (e.g., 0.0) in some Isaac Sim versions/configurations. Calling int(cuda_device) directly on a float string like "0.0" raises ValueError. A round-trip through float makes this robust without any behaviour change for integer values:
| cuda_device = sim_context.get_setting("/physics/cudaDevice") | |
| device_id = max(0, int(float(cuda_device)) if cuda_device is not None else 0) |
| env_cfg = parse_env_cfg(task_name, device=device, num_envs=num_envs) | ||
| # apply physics preset override before creating the environment | ||
| if physics_preset_name is not None: | ||
| # parse_env_cfg already resolved PresetCfg wrappers to their default, | ||
| # so we load the raw config to retrieve the named preset. | ||
| raw_cfg = load_cfg_from_registry(task_name, "env_cfg_entry_point") | ||
| # Unwrap if the top-level cfg is itself a PresetCfg wrapper. | ||
| raw_env_cfg = raw_cfg | ||
| if ( | ||
| not isinstance(raw_cfg, dict) | ||
| and hasattr(raw_cfg, "__dataclass_fields__") | ||
| and hasattr(raw_cfg, "default") | ||
| and not hasattr(type(raw_cfg), "class_type") | ||
| ): | ||
| raw_env_cfg = raw_cfg.default | ||
| # Apply the named preset to all preset wrappers in the config tree | ||
| # (e.g. sim.physics, scene.contact_forces, ...), not just sim.physics. | ||
| apply_named_preset(env_cfg, raw_env_cfg, physics_preset_name) |
There was a problem hiding this comment.
Config loaded twice per Newton test run
parse_env_cfg (line 325) internally calls load_cfg_from_registry, and then a second call to load_cfg_from_registry is made here (line 330) to obtain the unresolved config for preset application. For large, complex environments this doubles the module-import and object-construction cost every time a Newton test runs.
A cleaner approach would be to either:
- Extend
parse_env_cfgto accept an optionalphysics_preset_nameargument and apply the preset before returning, or - Load the raw config once and derive the resolved config from it (by calling
_resolve_presets_to_defaulton a copy).
This does not affect correctness, but is worth addressing before this pattern is repeated for other preset types.
| if hasattr(raw_value, preset_name): | ||
| resolved = getattr(raw_value, preset_name) | ||
| setattr(env_cfg, field_name, resolved) | ||
| apply_named_preset(resolved, resolved, preset_name) |
There was a problem hiding this comment.
Recursive call mutates the original preset object in-place
When a preset is found and applied, the recursion passes resolved as both env_cfg and raw_cfg:
apply_named_preset(resolved, resolved, preset_name)Because resolved is the same object reference that lives inside raw_value.<preset_name>, any in-place mutations made by the recursive call (e.g., resolving nested PresetCfg fields on resolved) are written back to the original config class attribute. In tests this is harmless because each test call reloads the config via load_cfg_from_registry, but if the same resolved config object were reused across multiple calls the mutations would accumulate unexpectedly.
The same pattern appears on line 99 (old-style preset branch). Consider passing a shallow copy of resolved to isolate mutations:
import copy
apply_named_preset(resolved, copy.copy(resolved), preset_name)…-public into add-newton-env-tests
Signed-off-by: Kelly Guo <kellyg@nvidia.com>
…ab-public into add-newton-env-tests
AntoineRichard
left a comment
AntoineRichard
left a comment
There was a problem hiding this comment.
I couldn't get Cassie to train yet, but I think with the attached settings Ant and Humanoid do train well. The Ant mods also work well with PhysX.
| # Normalize "cuda" -> "cuda:<id>" so torch.cuda.set_device() gets an explicit | ||
| # device index. PhysxManager does the same via /physics/cudaDevice setting. | ||
| device = PhysicsManager._device | ||
| if "cuda" in device and ":" not in device: | ||
| cuda_device = sim_context.get_setting("/physics/cudaDevice") | ||
| device_id = max(0, int(cuda_device) if cuda_device is not None else 0) | ||
| PhysicsManager._device = f"cuda:{device_id}" | ||
|
|
There was a problem hiding this comment.
Can't we handle that at the config level?
There was a problem hiding this comment.
@configclass
class AntPhysicsCfg(PresetCfg):
default: PhysxCfg = PhysxCfg()
physx: PhysxCfg = PhysxCfg()
newton: NewtonCfg = NewtonCfg(
solver_cfg=MJWarpSolverCfg(
njmax=45,
nconmax=25,
cone="pyramidal",
integrator="implicitfast",
impratio=1,
),
num_substeps=1,
debug_mode=False,
)
| "body": ImplicitActuatorCfg( | ||
| joint_names_expr=[".*"], | ||
| stiffness=0.0, | ||
| damping=0.0, | ||
| damping=0.5, | ||
| effort_limit_sim=30.0, | ||
| ), | ||
| }, |
There was a problem hiding this comment.
actuators={
"body": ImplicitActuatorCfg(
joint_names_expr=[".*"],
stiffness=0.0,
damping=0.1,
armature=0.02,
effort_limit_sim=15.0,
),
},There was a problem hiding this comment.
Joint ordering matters for humanoid:
physx_joint_gears: list = [
67.5000, # lower_waist
67.5000, # lower_waist
67.5000, # right_upper_arm
67.5000, # right_upper_arm
67.5000, # left_upper_arm
67.5000, # left_upper_arm
67.5000, # pelvis
45.0000, # right_lower_arm
45.0000, # left_lower_arm
45.0000, # right_thigh: x
135.0000, # right_thigh: y
45.0000, # right_thigh: z
45.0000, # left_thigh: x
135.0000, # left_thigh: y
45.0000, # left_thigh: z
90.0000, # right_knee
90.0000, # left_knee
22.5, # right_foot
22.5, # right_foot
22.5, # left_foot
22.5, # left_foot
]
newton_joint_gears: list = [
67.5000, # left_upper_arm
67.5000, # left_upper_arm
45.0000, # left_lower_arm
67.5000, # lower_waist
67.5000, # lower_waist
67.5000, # pelvis
45.0000, # left_thigh: x
135.0000, # left_thigh: y
45.0000, # left_thigh: z
90.0000, # left_knee
22.5, # left_foot
22.5, # left_foot
45.0000, # right_thigh: x
135.0000, # right_thigh: y
45.0000, # right_thigh: z
90.0000, # right_knee
22.5, # right_foot
22.5, # right_foot
67.5000, # right_upper_arm
67.5000, # right_upper_arm
45.0000, # right_lower_arm
]
joint_gears = {
"physx": physx_joint_gears,
"newton": newton_joint_gears,
}There was a problem hiding this comment.
This is also needed in the locomotion env:
# Resolve the joint gears based on the physics type, since they do not have the same joint ordering.
if isinstance(self.cfg.joint_gears, dict):
print(self.cfg.sim.physics)
if self.cfg.sim.physics.__class__.__name__ == "PhysxCfg":
joint_gears = self.cfg.joint_gears["physx"]
elif self.cfg.sim.physics.__class__.__name__ == "NewtonCfg":
joint_gears = self.cfg.joint_gears["newton"]
else:
raise ValueError(f"Invalid physics type: {self.cfg.sim.physics}")
else:
joint_gears = self.cfg.joint_gears
self.joint_gears = torch.tensor(joint_gears, dtype=torch.float32, device=self.sim.device)There was a problem hiding this comment.
I could make the material API for newton, we've had that working in dev/newton.
| njmax=200, | ||
| nconmax=70, | ||
| impratio=10.0, | ||
| cone="elliptic", |
There was a problem hiding this comment.
Do we need elliptic for it to work. It usually makes the solver less stable.
…-public into add-newton-env-tests
kellyguo11
requested review from
hhansen-bdai,
matthewtrepte,
ooctipus,
ossamaAhmed and
pascal-roth
as code owners
| self.joint_gears = torch.tensor(self.cfg.joint_gears, dtype=torch.float32, device=self.sim.device) | ||
| # Resolve the joint gears based on the physics type, since they do not have the same joint ordering. | ||
| if isinstance(self.cfg.joint_gears, dict): | ||
| print(self.cfg.sim.physics) |
There was a problem hiding this comment.
Debug print statement left in production code
print(self.cfg.sim.physics) on this line is a debug statement that was accidentally left in. It will produce noise in every run where joint_gears is a dict, polluting logs with the full physics config object repr.
| print(self.cfg.sim.physics) | |
| # Resolve the joint gears based on the physics type, since they do not have the same joint ordering. | |
| if isinstance(self.cfg.joint_gears, dict): |
| if self.cfg.sim.physics.__class__.__name__ == "PhysxCfg": | ||
| joint_gears = self.cfg.joint_gears["physx"] | ||
| elif self.cfg.sim.physics.__class__.__name__ == "NewtonCfg": | ||
| joint_gears = self.cfg.joint_gears["newton"] | ||
| else: | ||
| raise ValueError(f"Invalid physics type: {self.cfg.sim.physics}") |
There was a problem hiding this comment.
Fragile physics-type dispatch via __class__.__name__ string comparison
Comparing class names as strings breaks silently if either config class is ever renamed, subclassed, or imported under a different alias. Using isinstance() is the idiomatic, refactor-safe approach.
For example:
from isaaclab_physx.physics import PhysxCfg
from isaaclab_newton.physics import NewtonCfg
if isinstance(self.cfg.sim.physics, PhysxCfg):
joint_gears = self.cfg.joint_gears["physx"]
elif isinstance(self.cfg.sim.physics, NewtonCfg):
joint_gears = self.cfg.joint_gears["newton"]
else:
raise ValueError(f"Invalid physics type: {self.cfg.sim.physics}")This also properly handles subclasses of PhysxCfg or NewtonCfg without additional changes.
github-actions
bot
added
the
documentation
Adds environment tests for supported Newton environments running with Newton backend. Removes explicit definition of ls_iterations and ls_parallel in Newton cfgs as they are no longer needed. Fixes goal orientation matching for in hand manipulation environment. Fixes franka cabinet initialization orientation. Fixes reach environment training - uses a box for table as AssetBase doesn't work with Newton. Fixes mass randomization indexing. <!-- As you go through the list, delete the ones that are not applicable. --> - Bug fix (non-breaking change which fixes an issue) - New feature (non-breaking change which adds functionality) Please attach before and after screenshots of the change if applicable. <!-- Example: | Before | After | | ------ | ----- | | _gif/png before_ | _gif/png after_ | To upload images to a PR -- simply drag and drop an image while in edit mode and it should upload the image directly. You can then paste that source into the above before/after sections. --> - [x] I have read and understood the [contribution guidelines](https://isaac-sim.github.io/IsaacLab/main/source/refs/contributing.html) - [x] I have run the [`pre-commit` checks](https://pre-commit.com/) with `./isaaclab.sh --format` - [x] I have made corresponding changes to the documentation - [x] My changes generate no new warnings - [ ] I have added tests that prove my fix is effective or that my feature works - [ ] I have updated the changelog and the corresponding version in the extension's `config/extension.toml` file - [ ] I have added my name to the `CONTRIBUTORS.md` or my name already exists there <!-- As you go through the checklist above, you can mark something as done by putting an x character in it For example, - [x] I have done this task - [ ] I have not done this task --> --------- Signed-off-by: Kelly Guo <kellyg@nvidia.com> Co-authored-by: Antoine Richard <antoiner@nvidia.com>
2 participants
Description
Adds environment tests for supported Newton environments running with Newton backend.
Removes explicit definition of ls_iterations and ls_parallel in Newton cfgs as they are no longer needed.
Fixes goal orientation matching for in hand manipulation environment.
Fixes franka cabinet initialization orientation.
Fixes reach environment training - uses a box for table as AssetBase doesn't work with Newton.
Fixes mass randomization indexing.
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