Fix AutoMate PhysX sizing and assembly ID validation#6038
Conversation
There was a problem hiding this comment.
🤖 Isaac Lab Review Bot
Summary
Clean, well-scoped bugfix that increases the PhysX GPU collision stack size from 2²⁶ (64 MB) to 2²⁷ (128 MB) for AutoMate assembly/disassembly environments only.
✅ Strengths
- Minimal and targeted — only affects AutoMate high-contact 128-env tasks, not global PhysX defaults
- Consistent — same change applied symmetrically to both
assembly_env_cfg.pyanddisassembly_env_cfg.py - Regression test included —
test_automate_cfg.pyis a lightweight config assertion that does not require Isaac Sim runtime - Changelog fragment — properly added under
changelog.d/ - Placement —
gpu_collision_stack_sizeis grouped with othergpu_max_*settings, maintaining readability
🔍 Observations
- The 2²⁷ value (128 MB) gives ~50-70% headroom above the reported 75-86 MB requirement — reasonable margin without being excessive.
- The test instantiates the full
*EnvCfgdataclass and checks.sim.physics.gpu_collision_stack_size— will catch accidental regressions or config refactors that drop this setting.
💡 Minor Suggestions (non-blocking)
- Consider adding an inline comment (like the existing
# Important for stable simulation.ongpu_max_num_partitions) explaining why 2²⁷ was chosen, e.g.# 128 MB; default 64 MB insufficient for 128-env high-contact scenes. - The test file could optionally use
@pytest.mark.parametrizeif more config assertions are added later, but for two assertions this is fine as-is.
Verdict
LGTM ✅ — No correctness or architectural concerns. CI passing where applicable (pre-commit ✓, build ✓). Ready to merge once remaining CI checks complete.
Reviewed at commit: fd307ba
There was a problem hiding this comment.
Code Review Summary
Overall Assessment: This is a clean, minimal fix for the PhysX GPU collision stack sizing issue in AutoMate environments, with a simple guard against running with placeholder assembly IDs.
Changes Since Last Review (3312209 → 8feda53)
The PR has been significantly simplified — the parse_assembly_id() helper, unit tests, documentation changes, and argparse type= integration have been removed in favor of a minimal inline check. This is now a much smaller, more focused PR.
Strengths
-
Properly scoped PhysX fix: The
gpu_collision_stack_size=2**27increase is applied only to AutoMate assembly and disassembly environments, avoiding side effects on other environments. The 2^27 value (~134 MB) provides good headroom over the reported 75-86 MB requirement. -
Minimal validation: Both
run_w_id.pyandrun_disassembly_w_id.pynow catch the common mistake of running with the documentation placeholderASSEMBLY_IDbefore simulation starts. The error message is clear and actionable. -
Changelog entry: Properly formatted changelog fragment covering both changes.
Minor Observations
-
The validation only catches the exact string
"ASSEMBLY_ID". This is intentional simplicity — it catches the most common user mistake (copy-pasting from docs without replacing the placeholder) without over-engineering. -
The changelog still mentions "docs to reject placeholder assembly IDs" but the actual docs changes from the previous revision have been removed. The changelog entry is slightly broader than what the code does now — may want to adjust "and docs" to just "run helpers". (Very minor, not blocking.)
Verification
The changes look correct:
- Both
assembly_env_cfg.pyanddisassembly_env_cfg.pyconsistently setgpu_collision_stack_size=2**27 - Both run helpers validate the placeholder identically
- No syntax or formatting issues
- PR is now 5 files with minimal, easy-to-review changes
Reviewed at 8feda53 (incremental update from 3312209) 🤖
Greptile SummaryThis PR increases the PhysX GPU collision stack size from the default
Confidence Score: 5/5Safe to merge; the change is a single-line additive config tweak in two closely mirrored files, matched by a regression test. Both env configs receive identical, well-motivated increases to the PhysX collision stack. The new test correctly walks the sim.physics.gpu_collision_stack_size attribute chain. The changelog fragment is in the right place. No logic is altered outside of memory sizing, and the test guards against future regression. No files require special attention. The test will simply fail to import in CI environments without Isaac Sim, which is consistent with existing project constraints rather than a new problem introduced here. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[AssemblyEnvCfg / DisassemblyEnvCfg] --> B[SimulationCfg]
B --> C[PhysxCfg]
C --> D["gpu_collision_stack_size = 2**27 (128 MiB)"]
C --> E["gpu_max_rigid_contact_count = 2**23"]
C --> F["gpu_max_rigid_patch_count = 2**23"]
C --> G["gpu_max_num_partitions = 1"]
D --> H["Prevents collision-stack overflow at 128 parallel environments"]
H --> I["test_automate_cfg.py asserts value stays at 2**27"]
Reviews (1): Last reviewed commit: "Fix AutoMate PhysX collision stack sizin..." | Re-trigger Greptile |
fd307ba to
3312209
Compare
3312209 to
8feda53
Compare
…ation (#6048) ## Summary - Cherry-pick #6038 from develop into release/3.0.0-beta2 - Increase AutoMate PhysX GPU collision stack sizing for assembly/disassembly tasks - Reject the literal ASSEMBLY_ID placeholder before resolving AutoMate asset paths ## Validation - git diff --check refs/remotes/upstream/release/3.0.0-beta2...HEAD - python3 -m py_compile source/isaaclab_tasks/isaaclab_tasks/direct/automate/run_w_id.py source/isaaclab_tasks/isaaclab_tasks/direct/automate/run_disassembly_w_id.py source/isaaclab_tasks/isaaclab_tasks/direct/automate/assembly_env_cfg.py source/isaaclab_tasks/isaaclab_tasks/direct/automate/disassembly_env_cfg.py
Summary
Rationale
The reported Windows beta2 disassembly run for assembly_id 00032 asks PhysX for a collision stack of roughly 75-86 MB, above the default 2**26 bytes. This keeps the memory change scoped to AutoMate's high-contact 128-env tasks instead of raising the global PhysX default for unrelated environments.
A second reported DGX Spark command passed the literal docs placeholder
ASSEMBLY_ID, which was written into the task config and only failed later as a missing remote USD path. The helpers now fail fast with an argparse message before launching Kit.Validation
python3 tools/changelog/cli.py check developpython3 -m py_compile source/isaaclab_tasks/isaaclab_tasks/contrib/automate/run_w_id.py source/isaaclab_tasks/isaaclab_tasks/contrib/automate/run_disassembly_w_id.py source/isaaclab_tasks/test/contrib/test_automate_cfg.py source/isaaclab_tasks/isaaclab_tasks/contrib/automate/disassembly_env_cfg.py source/isaaclab_tasks/isaaclab_tasks/contrib/automate/assembly_env_cfg.py.venv/bin/python -m pytest source/isaaclab_tasks/test/contrib/test_automate_cfg.pyuvx ruff==0.14.10 check --fix source/isaaclab_tasks/test/contrib/test_automate_cfg.py source/isaaclab_tasks/isaaclab_tasks/contrib/automate/run_w_id.py source/isaaclab_tasks/isaaclab_tasks/contrib/automate/run_disassembly_w_id.py source/isaaclab_tasks/isaaclab_tasks/contrib/automate/disassembly_env_cfg.py source/isaaclab_tasks/isaaclab_tasks/contrib/automate/assembly_env_cfg.pyuvx ruff==0.14.10 format source/isaaclab_tasks/test/contrib/test_automate_cfg.py source/isaaclab_tasks/isaaclab_tasks/contrib/automate/run_w_id.py source/isaaclab_tasks/isaaclab_tasks/contrib/automate/run_disassembly_w_id.py source/isaaclab_tasks/isaaclab_tasks/contrib/automate/disassembly_env_cfg.py source/isaaclab_tasks/isaaclab_tasks/contrib/automate/assembly_env_cfg.pygit diff --check.venv/bin/python source/isaaclab_tasks/isaaclab_tasks/contrib/automate/run_w_id.py --assembly_id=ASSEMBLY_ID --train --max_iterations 10exits at argparse with the placeholder-specific message and does not mutate the task configCould not run the full PhysX/Kit repro locally because this machine's uv environment does not have Isaac Sim/Kit on PYTHONPATH; the command fails before simulation with Isaac Sim not installed.