Draft: deformables#6202
Conversation
Greptile SummaryThis draft PR adds rendering correctness tests for a Franka cloth deformable environment, extending the existing
Confidence Score: 4/5Safe to merge as a draft — changes are test-only with no production code impact. The change is purely additive test infrastructure. The kit-based deformable test follows established patterns and should work correctly. The style/quality concerns — all kitless combinations being pre-skipped, @configclass classes defined inside a repeatedly-called function, and the 500-step CI warm-up cost — are worth addressing before promoting from draft but do not break anything currently. source/isaaclab_tasks/test/rendering_test_utils.py — specifically _make_franka_cloth_camera_env_cfg (inline configclass definitions) and DEFORMABLE_KITLESS_PHYSICS_RENDERER_AOV_COMBINATIONS (all entries skipped). Important Files Changed
Sequence Diagram%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
participant PT as pytest parametrize
participant TRD as rendering_test_deformable
participant CFG as _make_franka_cloth_camera_env_cfg
participant OV as _apply_overrides_to_env_cfg
participant ENV as ManagerBasedRLEnv
participant CAM as tiled_camera
participant VAL as validate_camera_outputs
PT->>TRD: (physics_backend, renderer, data_type)
TRD->>CFG: data_type
CFG-->>TRD: "TestFrankaClothCameraEnvCfg (inline @configclass)"
TRD->>OV: "presets=newton_mjwarp_vbd,{renderer}"
OV-->>TRD: patched env_cfg
TRD->>ENV: ManagerBasedRLEnv(env_cfg)
loop 500 steps
TRD->>ENV: env.step(zero_actions)
end
TRD->>CAM: data.output[data_type]
CAM-->>TRD: tensor
TRD->>VAL: "{data_type: tensor}"
VAL-->>TRD: pass / fail vs golden image
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
participant PT as pytest parametrize
participant TRD as rendering_test_deformable
participant CFG as _make_franka_cloth_camera_env_cfg
participant OV as _apply_overrides_to_env_cfg
participant ENV as ManagerBasedRLEnv
participant CAM as tiled_camera
participant VAL as validate_camera_outputs
PT->>TRD: (physics_backend, renderer, data_type)
TRD->>CFG: data_type
CFG-->>TRD: "TestFrankaClothCameraEnvCfg (inline @configclass)"
TRD->>OV: "presets=newton_mjwarp_vbd,{renderer}"
OV-->>TRD: patched env_cfg
TRD->>ENV: ManagerBasedRLEnv(env_cfg)
loop 500 steps
TRD->>ENV: env.step(zero_actions)
end
TRD->>CAM: data.output[data_type]
CAM-->>TRD: tensor
TRD->>VAL: "{data_type: tensor}"
VAL-->>TRD: pass / fail vs golden image
Reviews (1): Last reviewed commit: "draft" | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| DEFORMABLE_KITLESS_PHYSICS_RENDERER_AOV_COMBINATIONS = [ | ||
| *_make_sensor_data_type_params("newton", "ovrtx", extra_marks=(_OVRTX_NEWTON_DEFORMABLE_SKIP,)), | ||
| *_make_sensor_data_type_params("newton", "newton", extra_marks=(_NEWTON_WARP_DEFORMABLE_SKIP,)), | ||
| ] |
There was a problem hiding this comment.
Both entries in DEFORMABLE_KITLESS_PHYSICS_RENDERER_AOV_COMBINATIONS carry a skip mark (_OVRTX_NEWTON_DEFORMABLE_SKIP for newton+ovrtx and _NEWTON_WARP_DEFORMABLE_SKIP for newton+newton), so test_rendering_deformable_kitless.py will collect 14 test IDs that are all skipped — no case actually exercises the code path today. If this is intentional scaffolding for follow-up work, a comment or xfail to that effect would make the intent clearer to future readers and CI triage.
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!
| def _make_franka_cloth_camera_env_cfg(data_type: str): | ||
| """Create a test-local Franka cloth camera env cfg without exposing a production task.""" | ||
| import isaaclab.sim as sim_utils | ||
| from isaaclab.envs import mdp as env_mdp | ||
| from isaaclab.managers import ObservationGroupCfg as ObsGroup | ||
| from isaaclab.managers import ObservationTermCfg as ObsTerm | ||
| from isaaclab.managers import SceneEntityCfg | ||
| from isaaclab.sensors import CameraCfg | ||
| from isaaclab.utils.configclass import configclass | ||
|
|
||
| from isaaclab_tasks.core.lift.config.franka_soft.franka_cloth_env_cfg import FrankaClothEnvCfg, FrankaClothSceneCfg | ||
| from isaaclab_tasks.utils.presets import MultiBackendRendererCfg | ||
|
|
||
| @configclass | ||
| class TestFrankaClothCameraSceneCfg(FrankaClothSceneCfg): | ||
| """Franka cloth scene with a test-only camera sensor.""" | ||
|
|
||
| tiled_camera: CameraCfg = CameraCfg( | ||
| prim_path="/World/envs/env_.*/Camera", | ||
| offset=CameraCfg.OffsetCfg( | ||
| pos=(0.85, -0.55, 0.42), | ||
| rot=(0.52, 0.18, 0.27, 0.79), | ||
| convention="opengl", | ||
| ), | ||
| data_types=[data_type], | ||
| spawn=sim_utils.PinholeCameraCfg(clipping_range=(0.01, 2.5)), | ||
| width=100, | ||
| height=100, | ||
| renderer_cfg=MultiBackendRendererCfg(), | ||
| ) | ||
|
|
||
| @configclass | ||
| class TestFrankaClothCameraObservationsCfg: | ||
| """Image-only observations for the local rendering test env.""" | ||
|
|
||
| @configclass | ||
| class PolicyCfg(ObsGroup): | ||
| image = ObsTerm( | ||
| func=env_mdp.image, | ||
| params={"sensor_cfg": SceneEntityCfg("tiled_camera"), "data_type": data_type, "permute": True}, | ||
| ) | ||
|
|
||
| def __post_init__(self) -> None: | ||
| self.enable_corruption = False | ||
| self.concatenate_terms = True | ||
|
|
||
| policy: ObsGroup = PolicyCfg() | ||
|
|
||
| @configclass | ||
| class TestFrankaClothCameraEnvCfg(FrankaClothEnvCfg): | ||
| """Test-only camera variant of ``Isaac-Lift-Cloth-Franka``.""" | ||
|
|
||
| scene: TestFrankaClothCameraSceneCfg = TestFrankaClothCameraSceneCfg( | ||
| num_envs=4, env_spacing=2.5, replicate_physics=True | ||
| ) | ||
| observations: TestFrankaClothCameraObservationsCfg = TestFrankaClothCameraObservationsCfg() | ||
|
|
||
| def __post_init__(self) -> None: | ||
| super().__post_init__() | ||
| self.commands.deformable_pose.debug_vis = False | ||
| self.events.reset_deformable.params["position_range"] = { | ||
| "x": (0.0, 0.0), | ||
| "y": (0.0, 0.0), | ||
| "z": (0.0, 0.0), | ||
| } | ||
|
|
||
| return TestFrankaClothCameraEnvCfg() |
There was a problem hiding this comment.
@configclass classes defined inside a function called once per parametrized case
_make_franka_cloth_camera_env_cfg is invoked once for each of the 7 parametrized data_type values in a test session. Each call re-executes the @configclass decorator on three classes (TestFrankaClothCameraSceneCfg, TestFrankaClothCameraObservationsCfg, TestFrankaClothCameraEnvCfg) whose names are identical across invocations. If @configclass registers class names in any module-level registry (as some configclass frameworks do for serialisation/deserialization), repeated registration under the same name can silently overwrite entries or raise an error. Other rendering helpers in this file use pre-defined imported cfg classes (e.g. CartpoleCameraEnvCfg, ShadowHandCameraEnvCfg) to avoid this. Extracting the three inner classes to module level — parameterising them at construction time — would remove the risk.
| env = ManagerBasedRLEnv(env_cfg) | ||
|
|
||
| zero_actions = torch.zeros(env.num_envs, env.action_manager.total_action_dim, device=env.device) | ||
| for _ in range(500): |
There was a problem hiding this comment.
All other rendering helpers (rendering_test_cartpole, rendering_test_shadow_hand, rendering_test_dexsuite_kuka) capture the golden image immediately after environment creation without any simulation steps. The 500-step loop here adds substantial CI wall-clock time (multiplied across 7 data types × flaky reruns). If the cloth genuinely needs to settle before a stable image can be captured, it would help to document the minimum number of steps required and whether a smaller count (e.g. 50–100) achieves the same goal.
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!
| env_cfg.scene.num_envs = 4 | ||
|
|
||
| if renderer == "ovrtx_renderer": | ||
| _redirect_ovrtx_renderer_log_to_stdout(env_cfg) | ||
|
|
||
| test_name = "franka_cloth_deformable" |
There was a problem hiding this comment.
num_envs=4 assignment — the scene cfg is already constructed with num_envs=4 inside _make_franka_cloth_camera_env_cfg. The identical re-assignment here adds noise without effect.
| env_cfg.scene.num_envs = 4 | |
| if renderer == "ovrtx_renderer": | |
| _redirect_ovrtx_renderer_log_to_stdout(env_cfg) | |
| test_name = "franka_cloth_deformable" | |
| if renderer == "ovrtx_renderer": | |
| _redirect_ovrtx_renderer_log_to_stdout(env_cfg) | |
| test_name = "franka_cloth_deformable" |
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!
1 participant
Description
Please include a summary of the change and which issue is fixed. Please also include relevant motivation and context.
List any dependencies that are required for this change.
Fixes # (issue)
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