Fix Newton deformables demo self-contact configuration

[mmichelis]

Description

Enable particle self-contact in the Newton VBD solver config used by the deformables demo, and tune the contact radius/margin so particles don't inter-penetrate.

Fixes NVBUG 6201420

Type of change

• Bug fix (non-breaking change which fixes an issue)

Checklist

• I have read and understood the contribution guidelines
• I have run the pre-commit checks with ./isaaclab.sh --format
• I have made corresponding changes to the documentation
• 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

[isaaclab-review-bot]

Isaac Lab Review Bot 🤖

Thanks for the contribution! I've analyzed PR #5744 and have the following observations:

Summary

This PR enables particle self-contact in the Newton VBD solver configuration for the deformables demo, which should resolve the particle inter-penetration issue (NVBUG 6201420).

---

Findings

1. [Low] Consider documenting the parameter tuning rationale

File: scripts/demos/deformables.py (lines 245-252)

The new parameters (particle_self_contact_radius=0.0001, particle_self_contact_margin=0.1, iterations=5) appear to be empirically tuned values. Consider adding a brief comment explaining:

• Why iterations was reduced from 10 to 5
• The relationship between particle_self_contact_radius and particle_self_contact_margin
• What scale/units these values assume (e.g., are they in meters?)

This helps future maintainers understand the tuning decisions.

physics_cfg = NewtonCfg(
    solver_cfg=VBDSolverCfg(
        iterations=5,  # Consider adding: "reduced from 10 for performance; sufficient with self-contact enabled"
        particle_enable_self_contact=True,
        particle_self_contact_radius=0.0001,  # meters? document units
        particle_self_contact_margin=0.1,  # relative margin for contact detection
    ),
    num_substeps=4,
)

2. [Info] PR contains squashed commits from other PRs

This PR appears to contain commits from multiple unrelated PRs that were squashed into the branch:

• #5713 (test environment speed improvements)
• #5714 (docs release branch updates)
• #5716 (teleop replay agent refactoring)
• Automated changelog compilations

While this is likely due to branch synchronization with the base branch, it makes the PR diff larger than the actual changes for this fix (32 files changed vs. the 1 file that contains the actual fix).

3. [Info] Missing changelog fragment for the deformables fix

The PR description indicates this is a bug fix, but I don't see a new changelog fragment specifically for the deformables self-contact fix. The existing changelog entries in this PR are from the squashed/merged commits.

Consider adding a changelog fragment like:

source/isaaclab_newton/changelog.d/fix-deformables-self-contact.rst

Fixed
^^^^^

* Fixed particle self-contact configuration in the Newton deformables demo to prevent inter-penetration.

4. [Low] Test coverage for the deformables demo configuration

The scripts/demos/deformables.py file doesn't appear to have dedicated unit tests. While the environment smoke tests may exercise this code path, consider whether the specific Newton VBD solver configuration should have explicit validation tests to catch configuration regressions.

---

CI Status

• ⚠️ Check changelog fragments: failing (expected - needs changelog fragment for this fix)
• ✅ Pre-commit checks: passing
• ⏳ Installation tests: pending

---

Overall

The core fix is straightforward and targeted. The main suggestions are around documentation and test coverage to help with long-term maintainability.---

Update (99def4e): Re-reviewed after new commits.

✅ Finding #3 addressed: Changelog fragment source/isaaclab_newton/changelog.d/fix-deformables-self-contact.rst has been added with the exact fix description I suggested.

The remaining findings are informational/low priority suggestions. The PR now looks good to merge:

• Core fix is correct: particle_enable_self_contact=True with appropriate radius/margin values
• Proper changelog entry included
• CI should pass once changelog check reruns

[greptile-apps]

Greptile Summary

This PR fixes the Newton VBD deformables demo by enabling particle self-contact and tuning the contact radius/margin. It also ships improvements to teleop_replay_agent.py (per-render stats scaling, an env_cfg block in the JSON report, "Kit" → "simulator" terminology), plus test-utility improvements that force interval events to fire on the first step.

• Deformables fix (scripts/demos/deformables.py): Replaces a bare VBDSolverCfg(iterations=10) call with self-contact enabled and tuned radius/margin values to prevent particle inter-penetration (NVBUG 6201420).
• Teleop stats (teleop_replay_agent.py): cpu_frame_time_ms and fps now report on a per-render basis; a new env_cfg block makes the JSON report self-contained. _STATS_SCHEMA_VERSION stays at 1 despite the semantic shift in metric units.
• Test utilities (env_test_utils.py): New _force_interval_events_to_fire_immediately helper and a force_interval_events flag let the env-smoke tests exercise interval event terms within a short 20-step budget.

Confidence Score: 4/5

Safe to merge; the deformables bug fix and teleop improvements are well-scoped and carry no breaking runtime risk.

The deformables fix is a straightforward config change targeting a specific solver bug. The teleop stats changes are additive and contain good defensive null-handling. The only meaningful concern is that cpu_frame_time_ms and fps fields silently change units while schema_version stays at 1, which may surprise downstream consumers of the JSON report.

teleop_replay_agent.py (schema version vs. semantic change) and env_test_utils.py (dead exception handler)

Important Files Changed

Filename	Overview
scripts/demos/deformables.py	Targeted Newton VBD solver config fix: enables particle self-contact, halves iteration count from 10 to 5, and sets contact radius (0.0001) and margin (0.1). Core change is clean.
scripts/environments/teleoperation/teleop_replay_agent.py	Adds per-render CPU/FPS stats scaling and an env_cfg block to the replay report. Logic is sound but schema_version stays at 1 despite a silent semantic change to existing metric fields.
source/isaaclab_tasks/test/env_test_utils.py	Adds force_interval_events helper and reduces default num_steps from 100 to 20. Dead try/except AttributeError around a three-argument getattr is harmless but misleading.
source/isaaclab_tasks/test/test_environments.py	Passes force_interval_events=True to ensure interval event terms fire on the first step; straightforward call-site update.

Comments Outside Diff (1)

1. scripts/environments/teleoperation/teleop_replay_agent.py, line 393 (link)

   Schema version not bumped after semantic change to reported metrics

   _STATS_SCHEMA_VERSION stays at 1, but the values in cpu_frame_time_ms and fps now carry different units: per-render instead of per-env.step. Any downstream consumer (CI dashboards, regression scripts, ML tooling) that ingested the old per-step numbers will silently receive values that are scaled by render_interval / decimation without any signal that the semantics changed. A version bump (or at minimum a new units field) would let consumers detect the change and adapt.

_{Reviews (1): Last reviewed commit: "Fix: Tune parameters for proper particle..." | Re-trigger Greptile}

[greptile-apps]

Redundant except AttributeError around a three-argument getattr

The three-argument form getattr(obj, name, default) already silently swallows AttributeError raised inside property getters and returns the default — it never raises AttributeError to the caller. The try/except AttributeError block is therefore dead code. More importantly, if any property getter on events_cfg raises a non-AttributeError exception (e.g. RuntimeError, TypeError), that exception will propagate uncaught and abort the test-setup loop unexpectedly. Either remove the dead handler or broaden it to except Exception to match the intent.

[greptile-apps]

Stale "Kit" reference in the changelog entry

The Changed-section description still says "the number of Kit renders per env.step", while every other occurrence of "Kit" in the same PR was replaced with "simulator". Minor inconsistency but worth aligning for a clean release note.

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!