Scene Data Provider Update

[daniela-hase]

Summary

• Introduces a new isaaclab.scene_data sub-package that consolidates SceneDataProvider, SceneDataBackend, and SceneDataFormat in a single import location.
• Breaking: Moves SceneDataProvider out of isaaclab.scene.scene_data_provider and SceneDataBackend / SceneDataFormat out of isaaclab.physics. All call sites in isaaclab_physx, isaaclab_newton,
  isaaclab_visualizers, isaaclab.sim, and tests are updated to the new path.
• Adds an API doc page (docs/source/api/lab/isaaclab.scene_data.rst) and a major-tier changelog fragment with migration guidance.

Motivation

Physics backends (isaaclab_physx, isaaclab_newton) need to subclass SceneDataBackend without pulling isaaclab.scene into the AppLauncher pre-launch import chain.

Migration

  # before
  from isaaclab.scene.scene_data_provider import SceneDataProvider
  from isaaclab.physics import SceneDataBackend, SceneDataFormat

  # after
  from isaaclab.scene_data import SceneDataProvider, SceneDataBackend, SceneDataFormat

Test plan

  - ./isaaclab.sh -f passes
  - ./isaaclab.sh -p -m pytest source/isaaclab/test/sim/test_newton_manager_visualization_state.py
  - Spot-check that PhysX and Newton backends import cleanly under AppLauncher
  - ./isaaclab.sh -d rebuilds docs without warnings on the new isaaclab.scene_data page

[isaaclab-review-bot]

Review Summary

This PR consolidates SceneDataProvider, SceneDataBackend, and SceneDataFormat into a new isaaclab.scene_data sub-package. The refactoring motivation is sound — isolating these types prevents circular import issues with AppLauncher's module-popping behavior during Kit initialization.

---

✅ Critical: Missing __init__.py for New Package (RESOLVED)

Update (417fa98): The missing __init__.py and __init__.pyi files have been added. The implementation uses lazy_export() from isaaclab.utils.module for lazy loading, which is consistent with the project's existing patterns. The type stub provides explicit exports for IDE support.

Original issue (now fixed)

The new isaaclab/scene_data/ directory did not include an __init__.py file, which would cause ModuleNotFoundError for package-level imports.

---

🟡 CI: Missing Changelog Fragment for isaaclab_visualizers

The PR modifies code in four packages but only provides changelog fragments for three:

• ✅ isaaclab — has changelog.d/daniela-move-scene-data.major.rst
• ✅ isaaclab_newton — has changelog.d/daniela-move-scene-data.rst
• ✅ isaaclab_physx — has changelog.d/daniela-move-scene-data.rst
• ❌ isaaclab_visualizers — still missing (modifies 4 visualizer files)

Please add source/isaaclab_visualizers/changelog.d/daniela-move-scene-data.rst similar to the newton/physx ones.

---

💡 Suggestion: Backward Compatibility Deprecation Warning

The changelog notes this as a breaking change with no deprecation period. For downstream users, consider adding a deprecation shim in the old locations (optional but would ease migration).

---

✅ Positive Observations

1. Documentation is comprehensive — API docs, feature docs, and concept docs all updated consistently
2. Migration guidance is clear — changelog fragment provides before/after import examples
3. All internal call sites updated — tests, backends, visualizers all use new paths
4. Docstrings updated — module docstring in scene_data_backend.py accurately explains why these types live separately from isaaclab.scene
5. Uses project conventions — lazy_export() pattern is consistent with other Isaac Lab modules

---

Verdict: The PR is nearly ready. Only the isaaclab_visualizers changelog fragment remains missing. From a code quality perspective, this is ready for maintainer review once that fragment is added.

---

📝 Re-review (a7e44c9)

Changes since last review (64541f3):

• Changed scene_data_provider.py import from absolute (from isaaclab.scene_data.scene_data_backend) to relative (from .scene_data_backend)

✅ This addresses the import style suggestion — relative imports are more idiomatic for intra-package imports and avoid ambiguity if the package is renamed or vendored.

No new issues found.

[greptile-apps]

Greptile Summary

This PR introduces isaaclab.scene_data as a new, standalone sub-package that consolidates SceneDataProvider, SceneDataBackend, and SceneDataFormat — previously split between isaaclab.scene.scene_data_provider and isaaclab.physics — into a single, import-chain-safe location. The motivation is to allow physics backends (isaaclab_physx, isaaclab_newton) to subclass SceneDataBackend before AppLauncher completes Kit initialisation without dragging isaaclab.scene into the pre-launch import chain.

• The new package follows the established lazy_export() + .pyi stub pattern used by every other isaaclab sub-package; all three public symbols are correctly declared and lazily loaded.
• All known call sites across isaaclab, isaaclab_physx, isaaclab_newton, isaaclab_visualizers, and the test suite are updated to the new path, and SceneDataBackend/SceneDataFormat are removed from physics/__init__.pyi.
• Changelog fragments, migration guidance, and API docs are all provided.

Confidence Score: 4/5

Safe to merge; the change is a clean package reorganisation with all known call sites updated and no runtime logic altered.

All moved files are functionally identical to their originals. The new scene_data package follows the existing lazy_export() pattern exactly, every import across the five affected packages is updated, and a grep confirms no stale references remain. The only note is a style preference for using a relative import inside scene_data_provider.py.

No files require special attention; scene_data_provider.py has a minor absolute-vs-relative import style point but nothing that would affect behaviour.

Important Files Changed

Filename	Overview
source/isaaclab/isaaclab/scene_data/init.py	New sub-package init.py that follows the lazy_export() pattern used by all other isaaclab packages; correct and minimal.
source/isaaclab/isaaclab/scene_data/init.pyi	Stub file correctly declares all three public names (SceneDataBackend, SceneDataFormat, SceneDataProvider) with accurate relative imports; consistent with other package stubs.
source/isaaclab/isaaclab/scene_data/scene_data_backend.py	Moved verbatim from physics/; only the doc-string module path reference was updated; logic unchanged.
source/isaaclab/isaaclab/scene_data/scene_data_provider.py	Moved from scene/; only the backend import path was updated. The absolute intra-package import works but a relative from .scene_data_backend import … would be more idiomatic.
source/isaaclab/isaaclab/physics/init.pyi	SceneDataBackend and SceneDataFormat correctly removed from physics stub; remaining exports are unaffected.
source/isaaclab_newton/isaaclab_newton/physics/newton_manager.py	Runtime import of SceneDataBackend/SceneDataFormat split into two lines; PhysicsManager etc. still imported from isaaclab.physics; correct.
source/isaaclab_physx/isaaclab_physx/physics/physx_manager.py	Same import split as newton_manager; clean and correct.
source/isaaclab/isaaclab/sim/simulation_context.py	SceneDataProvider runtime import updated to new path; no other changes in this file.
docs/source/api/lab/isaaclab.scene_data.rst	New API docs page follows the same RST structure as isaaclab.scene.rst; all three classes are documented.

_{Reviews (1): Last reviewed commit: "Merge branch 'develop' into dev/scene-da..." | Re-trigger Greptile}

[isaaclab-review-bot]

Review Summary (e4dd939)

This PR cleanly consolidates SceneDataProvider, SceneDataBackend, and SceneDataFormat into the new isaaclab.scene_data sub-package. Since the last review, the PR was rebased on develop (44 commits merged) and the relative import suggestion was addressed.

---

🟡 Outstanding: Missing Changelog Fragment for isaaclab_visualizers

The PR modifies code in four packages but only provides changelog fragments for three:

• ✅ isaaclab — has changelog.d/daniela-move-scene-data.major.rst
• ✅ isaaclab_newton — has changelog.d/daniela-move-scene-data.rst
• ✅ isaaclab_physx — has changelog.d/daniela-move-scene-data.rst
• ❌ isaaclab_visualizers — still missing (modifies 4 visualizer files)

Please add source/isaaclab_visualizers/changelog.d/daniela-move-scene-data.rst.

---

💡 Suggestion: Deprecation Shim for Old Import Paths

The changelog correctly marks this as a breaking change. For downstream users who may have code using the old paths, consider adding a deprecation shim in the old locations that raises DeprecationWarning with migration guidance. This is optional but would ease the transition.

Example for isaaclab/physics/__init__.py:

def __getattr__(name):
    if name in ("SceneDataBackend", "SceneDataFormat"):
        import warnings
        warnings.warn(
            f"{name} has moved to isaaclab.scene_data. "
            "Import from the new location.",
            DeprecationWarning,
            stacklevel=2
        )
        from isaaclab.scene_data import SceneDataBackend, SceneDataFormat
        return {"SceneDataBackend": SceneDataBackend, "SceneDataFormat": SceneDataFormat}[name]
    raise AttributeError(f"module {__name__!r} has no attribute {name!r}")

---

✅ Verification Complete

1. Import Architecture — lazy_export() pattern correctly isolates scene_data from the AppLauncher module-popping window
2. API Design — Consolidating these 3 tightly-coupled classes is the right call
3. Type Stubs — __init__.pyi correctly exports all public symbols
4. Documentation — API docs, concept docs, and feature docs all updated consistently
5. Internal Callsites — All 4 backend packages properly updated to new import paths
6. Relative Imports — scene_data_provider.py now uses from .scene_data_backend (addressed from prior feedback)

---

Verdict: Ready for merge once the isaaclab_visualizers changelog fragment is added. The maintainer approval from @pbarejko stands; this re-review confirms the refactoring is sound.

[isaaclab-review-bot]

Review Summary (e12a1ba)

This update since the last review addresses a bug where NewtonManager.update_visualization_state() was accessing the SimulationContext via an internal class variable instead of the proper singleton pattern.

---

🔴 Bug: Double Assignment Typo

File: source/isaaclab_newton/isaaclab_newton/physics/newton_manager.py, line 1572

sim = sim = SimulationContext.instance()  # ❌ typo: double assignment

Should be:

sim = SimulationContext.instance()

This is likely what's causing the pre-commit check to fail.

---

🟡 Behavioral Change: Removed allow_passthrough=False

The new code changes the call to sdp.get_transforms():

# Before (line ~1587)
sdp.get_transforms(
    cls._visualization_scene_data,
    mapping=cls._visualization_mapping,
    allow_passthrough=False,
)

# After
sdp.get_transforms(cls._scene_data, mapping=cls._scene_data_mapping)

The allow_passthrough parameter defaults to True, so this changes behavior. When True, the provider may skip conversion if the backend already provides data in the requested format. Was this intentional? If Newton visualization requires explicit conversion (to ensure fresh data), the original allow_passthrough=False may have been deliberate.

---

🟡 Return Type Mismatch with Base Class

File: source/isaaclab_newton/isaaclab_newton/physics/newton_manager.py

The base class PhysicsManager.get_scene_data_backend() has return type SceneDataBackend, but Newton now declares SceneDataBackend | None:

# PhysicsManager (base)
def get_scene_data_backend(cls) -> SceneDataBackend:

# NewtonManager (this PR)
def get_scene_data_backend(cls) -> SceneDataBackend | None:

If None is a valid return, the base class signature should also be updated for consistency. Otherwise, type checkers will flag this.

---

🟡 Still Missing: isaaclab_visualizers Changelog Fragment

From the previous review — the PR modifies 4 files in isaaclab_visualizers but no changelog fragment exists. Please add:

• source/isaaclab_visualizers/changelog.d/daniela-move-scene-data.rst

---

✅ Improvements in This Commit

1. Better Singleton Access — Using SimulationContext.instance() is more robust than accessing PhysicsManager._sim directly
2. Cleaner Variable Names — Renamed _visualization_scene_data → _scene_data and _visualization_mapping → _scene_data_mapping
3. Simplified Code — Removed multiline call formatting

---

Verdict: Fix the sim = sim = typo, verify the allow_passthrough change is intentional, and add the visualizers changelog. Pre-commit should pass once the typo is resolved.

---

Update (430d12b)

This commit addresses two issues from the previous review:

✅ Fixed: Double Assignment Typo

The sim = sim = SimulationContext.instance() has been corrected to sim = SimulationContext.instance(). Pre-commit checks should pass now.

✅ Added: Changelog Fragment

Added source/isaaclab_visualizers/changelog.d/daniela-move-scene-data.rst documenting:

• Updated configclass imports in visualizer modules to match lazy-import layout
• Updated test to use .torch accessor for Warp-backed camera data API

🟢 Bonus: Import Ordering

Imports in newton_manager.py have been alphabetically reordered (moved SimulationContext import to proper position).

---

Remaining open items from previous review:

• 🟡 Verify allow_passthrough behavioral change is intentional
• 🟡 Return type mismatch with base class PhysicsManager.get_scene_data_backend()

Overall: Good progress — the critical typo is fixed and changelog added. LGTM pending clarification on the two yellow items above.

---

Update (322a804)

This commit removes duplicate code in SimulationContext.update_visualizers():

✅ Fixed: Duplicate Marker Callback Dispatch

File: source/isaaclab/isaaclab/sim/simulation_context.py

The same block was called twice in succession:

# BEFORE: dispatched twice
if any(viz.supports_markers() for viz in self._visualizers):
    self.vis_marker_registry.dispatch_callbacks()

# Marker callbacks update VisualizationMarkers state; visualizer step()
# consumes that state later in this method.
if any(viz.supports_markers() for viz in self._visualizers):
    self.vis_marker_registry.dispatch_callbacks()  # ❌ duplicate

# AFTER: single dispatch (correct)
if any(viz.supports_markers() for viz in self._visualizers):
    self.vis_marker_registry.dispatch_callbacks()

This was likely a copy-paste error that could cause markers to update twice per frame, potentially wasting compute or causing subtle timing issues.

---

Remaining open items from previous review:

• 🟡 Verify allow_passthrough behavioral change is intentional
• 🟡 Return type mismatch with base class PhysicsManager.get_scene_data_backend()

Overall: Good cleanup. The duplicate marker dispatch removal is a sensible fix. LGTM pending the two yellow items above.

---

Update (ce27365)

This commit adds documentation for a bug fix in the Newton changelog fragment:

✅ Added: Bug Fix Documentation

File: source/isaaclab_newton/changelog.d/daniela-move-scene-data.rst

Added a "Fixed" section documenting:

• Fixed NewtonManager.update_visualization_state() retrieving the wrong simulation context
• Now uses SimulationContext.instance() instead of the stale PhysicsManager._sim reference

This properly documents the bug fix that was implemented in earlier commits, making the changelog complete for the Newton package.

---

Remaining open items:

• 🟡 Verify allow_passthrough behavioral change is intentional
• 🟡 Return type mismatch with base class PhysicsManager.get_scene_data_backend()

Overall: Good changelog hygiene. LGTM once the two yellow items above are clarified.

---

Update (e105d05)

Minor formatting fix:

✅ Fixed: Missing Newline at EOF

File: source/isaaclab_newton/changelog.d/daniela-move-scene-data.rst

Added the missing newline at end of file. This resolves any pre-commit linting warnings about files not ending with a newline character.

---

Remaining open items:

• 🟡 Verify allow_passthrough behavioral change is intentional
• 🟡 Return type mismatch with base class PhysicsManager.get_scene_data_backend()

Overall: Good housekeeping. LGTM once the two yellow items above are clarified.

---

Update (b32cc68)

⚠️ Note: This appears to be a significant merge or rebase that incorporates substantial changes unrelated to the original Scene Data Provider PR scope. The diff includes:

📦 Major Additions (Likely from main branch merge)

1. Newton Actuators Integration (articulation.py ~600+ lines changed)

   • New _apply_actuator_model_newton() fast path for Newton-native actuators
   • write_actuator_stiffness_to_sim() / write_actuator_damping_to_sim() methods
   • _has_newton_actuators / newton_actuator_adapter attributes
   • Integration with isaaclab_newton.actuators module

2. Contact Sensor Tests (test_contact_sensor.py - new ~500 lines)

   • Comprehensive contact sensor test suite
   • Tests for contact times, friction forces, contact positions

3. MCAP Replay Support (isaaclab_teleop package)

   • New mcap_record_path / mcap_replay_path parameters
   • Replay session support via SessionMode.REPLAY
   • Removed deprecated automation/xcr_replay.py

4. Newton Actuator Test Suite (test_newton_actuators_physx.py - new ~1100 lines)

   • IdealPD, DCMotor, DelayedPD, RemotizedPD, Neural MLP/LSTM equivalence tests
   • Domain randomization via events.py tests
   • Per-env reset isolation tests

5. Preset System Improvements (hydra.py / preset_cli.py)

   • Active-tree preset resolution (_resolve_active_presets)
   • Scoped nested preset choices
   • enumerate_task_presets() public helper

6. New Task Environments

   • Isaac-Assemble-Trocar-G129-Dex3-v0 (RLinf VLA fine-tuning)
   • OVPhysX preset for locomotion velocity tasks

7. Various Breaking Changes

   • Removed legacy teleop_devices accessor from PickPlaceGR1T2EnvCfg
   • Removed isaaclab_teleop.automation subpackage

🟢 Related to Original PR Scope

The changes to articulation.py include buffer management cleanup (removing pre-allocated CPU buffers, using wp.clone() instead of wp.copy()) which aligns with the Scene Data Provider refactoring theme.

📝 Review of Incremental Changes

Since this commit appears to be primarily a merge/rebase incorporating main branch changes, the original PR scope items remain:

Remaining open items from original review:

• 🟡 Verify allow_passthrough behavioral change is intentional
• 🟡 Return type mismatch with base class PhysicsManager.get_scene_data_backend()

---

Overall: This commit brings in many features from the main branch. The original Scene Data Provider changes appear intact. LGTM for the original PR scope pending the two yellow items above. The newly introduced features have their own changelog fragments and appear well-documented.

---

Update (cc08ac3)

This commit extends the Scene Data Provider refactoring to the isaaclab_ovphysx package:

✅ Import Path Update

File: source/isaaclab_ovphysx/isaaclab_ovphysx/physics/ovphysx_manager.py

# Before
from isaaclab.physics import PhysicsEvent, PhysicsManager, SceneDataBackend, SceneDataFormat

# After
from isaaclab.physics import PhysicsEvent, PhysicsManager
from isaaclab.scene_data import SceneDataBackend, SceneDataFormat

SceneDataBackend and SceneDataFormat are now imported from their new canonical location in isaaclab.scene_data (previously in isaaclab.physics). This aligns the OVPhysX manager with the new module structure introduced by this PR.

✅ Changelog Fragment Added

File: source/isaaclab_ovphysx/changelog.d/daniela-move-scene-data.rst (new)

Documents the import path change for the OVPhysX package, maintaining changelog consistency across all affected packages.

---

Remaining open items from previous reviews:

• 🟡 Verify allow_passthrough behavioral change is intentional
• 🟡 Return type mismatch with base class PhysicsManager.get_scene_data_backend()

Overall: Good incremental progress — OVPhysX now uses the new import paths with proper changelog documentation. LGTM pending the two yellow items above.

---

Update (4612dac)

This commit improves documentation build compatibility and adds docstrings to the scene data format structures:

✅ Sphinx Autodoc Compatibility

File: source/isaaclab/isaaclab/scene_data/scene_data_backend.py

Added a clever workaround for Sphinx documentation builds:

# Under Sphinx `autodoc_mock_imports`, `wp.struct` is a `_MockObject`
# that replaces the decorated class with another mock, hiding its docstring
# and fields from autodoc. Fall back to an identity decorator when warp is
# mocked so the documentation builds from the source classes directly.
if getattr(wp, "__sphinx_mock__", False):
    def wp_struct(cls):
        return cls
else:
    wp_struct = wp.struct

This allows the documentation generator to introspect the actual class definitions rather than warp mock objects, enabling proper autodoc output.

✅ Comprehensive Docstrings Added

All SceneDataFormat struct classes now have documentation:

• SceneDataFormat — Main class docstring explaining transform layouts
• Vec3_Quat — Separate position and quaternion arrays with field docs
• Vec3_Matrix33 — Separate position and rotation-matrix arrays
• Transform — Packed warp transforms (position + quaternion)
• Matrix44 — Packed 4×4 homogeneous transform matrices

Each struct field now includes units where applicable (e.g., [m] for positions).

---

Remaining open items from previous reviews:

• 🟡 Verify allow_passthrough behavioral change is intentional
• 🟡 Return type mismatch with base class PhysicsManager.get_scene_data_backend()

Overall: Excellent documentation improvement. The Sphinx mock detection is a clean solution for autodoc compatibility. LGTM pending the two yellow items above.

---

Update (d5a681c)

⚠️ Major merge commit — This includes an extensive deformable object refactoring and many other changes from main. The diff is very large (~3000+ lines of new code). Key changes:

📦 Deformable Object System Overhaul

1. New isaaclab_contrib.deformable module with comprehensive Newton VBD solver support:

   • DeformableObject and DeformableObjectData classes
   • VBDSolverCfg, CoupledMJWarpVBDSolverCfg, CoupledFeatherstoneVBDSolverCfg
   • Newton-specific material configs (NewtonDeformableBodyMaterialCfg, NewtonSurfaceDeformableBodyMaterialCfg)

2. Backend-specific deformable configs split between Newton and PhysX:

   • isaaclab_newton.sim.schemas.NewtonDeformableBodyPropertiesCfg
   • isaaclab_physx.sim.schemas.PhysxDeformableBodyPropertiesCfg
   • Deprecation warnings added for old generic names

3. New Franka soft-body lifting environments:

   • Isaac-Lift-Soft-Franka-v0 (volume deformable)
   • Isaac-Lift-Cloth-Franka-v0 (surface deformable/cloth)
   • Complete MDP (observations, rewards, terminations, events)

4. Newton particle mesh sync — new sync_particles_to_usd() for deformable visualization

5. Coupled solver support — rigid-deformable two-way coupling tests

🔄 Changes to Scene Data Provider (Original PR Scope)

File: source/isaaclab_newton/isaaclab_newton/physics/newton_manager.py

The update_visualization_state() method now accepts an optional scene_data_provider parameter:

# Before
def update_visualization_state(cls) -> None:

# After
def update_visualization_state(cls, scene_data_provider=None) -> None:

This allows callers to pass an explicit provider rather than fetching from SimulationContext. The method also now checks _backend_is_newton(scene_data_provider) which accepts the provider argument.

✅ Previously Raised Items Addressed

1. Return type mismatch — _backend_is_newton() now accepts scene_data_provider parameter, making the None return type meaningful (when provider has non-Newton backend)

2. Singleton pattern — The code now gracefully handles cases where SimulationContext.instance() returns None:

   sdp = scene_data_provider
   if sdp is None:
       sim = SimulationContext.instance()
       if sim is not None:
           sdp = sim.get_scene_data_provider()
   if sdp is None:
       return

🟢 Visualizer Updates (Related to PR Scope)

Files: viser_visualizer.py, rerun_visualizer.py, newton_visualizer.py

• Added focal_length support to Newton visualizer
• Improved camera FOV handling via _focal_length_to_vertical_fov_degrees()
• Browser auto-open disabled by default (URLs logged instead)
• open_browser config defaults changed from True to False
• Added _log_viewer_url() helper for consistent URL display
• verbose startup logging reduced to debug level

📝 New Changelog Fragments

Multiple new changelog files document the extensive changes:

• mtrepte-update-debug-viz.rst — visualizer state update fixes
• mym-deformable-backend-split.minor.rst — backend-specific deformable configs
• mym-deformable_experimental.minor.rst — deformable asset exports, cloning hooks
• esekkin-pr-a-newton-xfail.rst — removed stale xfail marker from Newton env tests

---

🟡 New Concern: Test Skip Added

File: source/isaaclab_visualizers/test/test_visualizer_cartpole_integration.py

@pytest.mark.skip(reason="ViewerGL frame motion is flaky on the current pinned Isaac Sim CI image.")
def test_cartpole_newton_visualizer_viewergl_rgb_motion(...)

This test was skipped due to CI flakiness. Should be tracked for follow-up to ensure it gets re-enabled once the CI image is updated.

---

Overall Assessment:

This is a massive update that goes well beyond the original "Scene Data Provider Update" scope. The PR now includes:

• Complete deformable object system with Newton VBD support
• Two new RL environments for soft-body manipulation
• Particle mesh synchronization for visualization
• Backend-specific config refactoring

Recommendation: Given the scope expansion, consider whether this should be split into multiple PRs for easier review and bisection. The Scene Data Provider changes are solid, but they're now interleaved with a major feature addition.

Previous yellow items:

• ✅ allow_passthrough — Less relevant now with the refactored update_visualization_state() flow
• ✅ Return type mismatch — Addressed with optional scene_data_provider parameter

LGTM for the Scene Data Provider portion. The deformable additions appear well-structured with proper changelog documentation.

[fatimaanes]

@daniela-hase Looks good overall! one tiny suggestion, worth adding a "Fixed" entry to the isaaclab_newton changelog so it's trackable.