Unverified Commit 39ce5ac4 authored by fyu-bdai's avatar fyu-bdai Committed by GitHub

Moves InteractiveScene entities from class to instance variables (#466)

# Description

<!--
Thank you for your interest in sending a pull request. Please make sure
to check the contribution guidelines.

Link: https://isaac-orbit.github.io/orbit/source/refs/contributing.html
-->

Currently, InteractiveScene declares several of its entities
(articulation, sensor, and rigid bodies) as class variables. This causes
the variables to be shared across **all** instances of the class, which
is not desired behavior if there is ever a case when `InteractiveScene`
is used more than once.

This fix changes the following: 

1. All class variables in `InteractiveScene` defined above the
constructor is now moved inside the constructor as instance variables.
2. Adds `@property` getters to access the entities. 
3. Rolls back the fix introduced in #380, which changes the `__del__`
magic method to clear out the class variables that persisted, but ended
up breaking #464.

Fixes #465 

<!-- As a practice, it is recommended to open an issue to have
discussions on the proposed pull request.
This makes it easier for the community to keep track of what is being
developed or added, and if a given feature
is demanded by more than one party. -->

## Type of change

<!-- As you go through the list, delete the ones that are not
applicable. -->

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

## Checklist

- [x] I have run the [`pre-commit` checks](https://pre-commit.com/) with
`./orbit.sh --format`
- [x] I have made corresponding changes to the documentation
- [x] My changes generate no new warnings
- [x] I have added tests that prove my fix is effective or that my
feature works
- [x] I have run all the tests with `./orbit.sh --test` and they pass
- [x] I have updated the changelog and the corresponding version in the
extension's `config/extension.toml` file
- [x] 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
-->
parent ba836722
[package]
# Note: Semantic Versioning is used: https://semver.org/
version = "0.15.2"
version = "0.15.3"
# Description
title = "ORBIT framework for Robot Learning"
......
Changelog
---------
0.15.3 (2024-03-21)
~~~~~~~~~~~~~~~~~~~
Added
^^^^^
* Added unit test to check that :class:`omni.isaac.orbit.scene.InteractiveScene` entity data is not shared between separate instances.
Fixed
^^^^^
* Moved class variables in :class:`omni.isaac.orbit.scene.InteractiveScene` to correctly be assigned as
instance variables.
* Removed custom ``__del__`` magic method from :class:`omni.isaac.orbit.scene.InteractiveScene`.
0.15.2 (2024-03-21)
~~~~~~~~~~~~~~~~~~~
......
......@@ -69,36 +69,6 @@ class InteractiveScene:
for more details.
"""
terrain: TerrainImporter | None = None
"""The terrain in the scene. If None, then the scene has no terrain.
Note:
We treat terrain separate from :attr:`extras` since terrains define environment origins and are
handled differently from other miscellaneous entities.
"""
articulations: dict[str, Articulation] = dict()
"""A dictionary of articulations in the scene."""
rigid_objects: dict[str, RigidObject] = dict()
"""A dictionary of rigid objects in the scene."""
sensors: dict[str, SensorBase] = dict()
"""A dictionary of the sensors in the scene, such as cameras and contact reporters."""
extras: dict[str, XFormPrimView] = dict()
"""A dictionary of miscellaneous simulation objects that neither inherit from assets nor sensors.
The keys are the names of the miscellaneous objects, and the values are the `XFormPrimView`_
of the corresponding prims.
As an example, lights or other props in the scene that do not have any attributes or properties that you
want to alter at runtime can be added to this dictionary.
Note:
These are not reset or updated by the scene. They are mainly other prims that are not necessarily
handled by the interactive scene, but are useful to be accessed by the user.
.. _XFormPrimView: https://docs.omniverse.nvidia.com/py/isaacsim/source/extensions/omni.isaac.core/docs/index.html#omni.isaac.core.prims.XFormPrimView
"""
def __init__(self, cfg: InteractiveSceneCfg):
"""Initializes the scene.
......@@ -107,6 +77,12 @@ class InteractiveScene:
"""
# store inputs
self.cfg = cfg
# initialize scene elements
self._terrain = None
self._articulations = dict()
self._rigid_objects = dict()
self._sensors = dict()
self._extras = dict()
# obtain the current stage
self.stage = omni.usd.get_context().get_stage()
# prepare cloner for environment replication
......@@ -149,12 +125,6 @@ class InteractiveScene:
global_paths=self._global_prim_paths,
)
def __del__(self):
"""Clear instances of registered assets and sensors."""
self.articulations.clear()
self.rigid_objects.clear()
self.sensors.clear()
def __str__(self) -> str:
"""Returns a string representation of the scene."""
msg = f"<class {self.__class__.__name__}>\n"
......@@ -201,11 +171,55 @@ class InteractiveScene:
@property
def env_origins(self) -> torch.Tensor:
"""The origins of the environments in the scene. Shape is (num_envs, 3)."""
if self.terrain is not None:
return self.terrain.env_origins
if self._terrain is not None:
return self._terrain.env_origins
else:
return self._default_env_origins
@property
def terrain(self) -> TerrainImporter | None:
"""The terrain in the scene. If None, then the scene has no terrain.
Note:
We treat terrain separate from :attr:`extras` since terrains define environment origins and are
handled differently from other miscellaneous entities.
"""
return self._terrain
@property
def articulations(self) -> dict[str, Articulation]:
"""A dictionary of articulations in the scene."""
return self._articulations
@property
def rigid_objects(self) -> dict[str, RigidObject]:
"""A dictionary of rigid objects in the scene."""
return self._rigid_objects
@property
def sensors(self) -> dict[str, SensorBase]:
"""A dictionary of the sensors in the scene, such as cameras and contact reporters."""
return self._sensors
@property
def extras(self) -> dict[str, XFormPrimView]:
"""A dictionary of miscellaneous simulation objects that neither inherit from assets nor sensors.
The keys are the names of the miscellaneous objects, and the values are the `XFormPrimView`_
of the corresponding prims.
As an example, lights or other props in the scene that do not have any attributes or properties that you
want to alter at runtime can be added to this dictionary.
Note:
These are not reset or updated by the scene. They are mainly other prims that are not necessarily
handled by the interactive scene, but are useful to be accessed by the user.
.. _XFormPrimView: https://docs.omniverse.nvidia.com/py/isaacsim/source/extensions/omni.isaac.core/docs/index.html#omni.isaac.core.prims.XFormPrimView
"""
return self._extras
"""
Operations.
"""
......@@ -218,12 +232,12 @@ class InteractiveScene:
Defaults to None (all instances).
"""
# -- assets
for articulation in self.articulations.values():
for articulation in self._articulations.values():
articulation.reset(env_ids)
for rigid_object in self.rigid_objects.values():
for rigid_object in self._rigid_objects.values():
rigid_object.reset(env_ids)
# -- sensors
for sensor in self.sensors.values():
for sensor in self._sensors.values():
sensor.reset(env_ids)
# -- flush physics sim view if called in extension mode
# this is needed when using PhysX GPU pipeline since the data needs to be sent to the underlying
......@@ -236,9 +250,9 @@ class InteractiveScene:
def write_data_to_sim(self):
"""Writes the data of the scene entities to the simulation."""
# -- assets
for articulation in self.articulations.values():
for articulation in self._articulations.values():
articulation.write_data_to_sim()
for rigid_object in self.rigid_objects.values():
for rigid_object in self._rigid_objects.values():
rigid_object.write_data_to_sim()
# -- flush physics sim view if called in extension mode
# this is needed when using PhysX GPU pipeline since the data needs to be sent to the underlying
......@@ -255,12 +269,12 @@ class InteractiveScene:
dt: The amount of time passed from last :meth:`update` call.
"""
# -- assets
for articulation in self.articulations.values():
for articulation in self._articulations.values():
articulation.update(dt)
for rigid_object in self.rigid_objects.values():
for rigid_object in self._rigid_objects.values():
rigid_object.update(dt)
# -- sensors
for sensor in self.sensors.values():
for sensor in self._sensors.values():
sensor.update(dt, force_recompute=not self.cfg.lazy_sensor_update)
"""
......@@ -274,7 +288,7 @@ class InteractiveScene:
The keys of the scene entities.
"""
all_keys = ["terrain"]
for asset_family in [self.articulations, self.rigid_objects, self.sensors, self.extras]:
for asset_family in [self._articulations, self._rigid_objects, self._sensors, self._extras]:
all_keys += list(asset_family.keys())
return all_keys
......@@ -289,11 +303,11 @@ class InteractiveScene:
"""
# check if it is a terrain
if key == "terrain":
return self.terrain
return self._terrain
all_keys = ["terrain"]
# check if it is in other dictionaries
for asset_family in [self.articulations, self.rigid_objects, self.sensors, self.extras]:
for asset_family in [self._articulations, self._rigid_objects, self._sensors, self._extras]:
out = asset_family.get(key)
# if found, return
if out is not None:
......@@ -323,11 +337,11 @@ class InteractiveScene:
# terrains are special entities since they define environment origins
asset_cfg.num_envs = self.cfg.num_envs
asset_cfg.env_spacing = self.cfg.env_spacing
self.terrain = asset_cfg.class_type(asset_cfg)
self._terrain = asset_cfg.class_type(asset_cfg)
elif isinstance(asset_cfg, ArticulationCfg):
self.articulations[asset_name] = asset_cfg.class_type(asset_cfg)
self._articulations[asset_name] = asset_cfg.class_type(asset_cfg)
elif isinstance(asset_cfg, RigidObjectCfg):
self.rigid_objects[asset_name] = asset_cfg.class_type(asset_cfg)
self._rigid_objects[asset_name] = asset_cfg.class_type(asset_cfg)
elif isinstance(asset_cfg, SensorBaseCfg):
# Update target frame path(s)' regex name space for FrameTransformer
if isinstance(asset_cfg, FrameTransformerCfg):
......@@ -337,7 +351,7 @@ class InteractiveScene:
updated_target_frames.append(target_frame)
asset_cfg.target_frames = updated_target_frames
self.sensors[asset_name] = asset_cfg.class_type(asset_cfg)
self._sensors[asset_name] = asset_cfg.class_type(asset_cfg)
elif isinstance(asset_cfg, AssetBaseCfg):
# manually spawn asset
if asset_cfg.spawn is not None:
......@@ -349,7 +363,7 @@ class InteractiveScene:
)
# store xform prim view corresponding to this asset
# all prims in the scene are Xform prims (i.e. have a transform component)
self.extras[asset_name] = XFormPrimView(asset_cfg.prim_path, reset_xform_properties=False)
self._extras[asset_name] = XFormPrimView(asset_cfg.prim_path, reset_xform_properties=False)
else:
raise ValueError(f"Unknown asset config type for {asset_name}: {asset_cfg}")
# store global collision paths
......
# Copyright (c) 2022-2024, The ORBIT Project Developers.
# All rights reserved.
#
# SPDX-License-Identifier: BSD-3-Clause
from __future__ import annotations
"""Launch Isaac Sim Simulator first."""
from omni.isaac.orbit.app import AppLauncher
# launch omniverse app
simulation_app = AppLauncher(headless=True).app
"""Rest everything follows."""
import unittest
import omni.isaac.orbit.sim as sim_utils
from omni.isaac.orbit.actuators import ImplicitActuatorCfg
from omni.isaac.orbit.assets import ArticulationCfg, AssetBaseCfg, RigidObjectCfg
from omni.isaac.orbit.scene import InteractiveScene, InteractiveSceneCfg
from omni.isaac.orbit.sensors import ContactSensorCfg
from omni.isaac.orbit.sim import build_simulation_context
from omni.isaac.orbit.terrains import TerrainImporterCfg
from omni.isaac.orbit.utils import configclass
from omni.isaac.orbit.utils.assets import ISAAC_NUCLEUS_DIR
@configclass
class MySceneCfg(InteractiveSceneCfg):
"""Example scene configuration."""
# terrain - flat terrain plane
terrain = TerrainImporterCfg(
prim_path="/World/ground",
terrain_type="plane",
)
# articulation
robot = ArticulationCfg(
prim_path="/World/Robot",
spawn=sim_utils.UsdFileCfg(usd_path=f"{ISAAC_NUCLEUS_DIR}/Robots/Simple/revolute_articulation.usd"),
actuators={
"joint": ImplicitActuatorCfg(),
},
)
# rigid object
rigid_obj = RigidObjectCfg(
prim_path="/World/RigidObj",
spawn=sim_utils.CuboidCfg(
size=(0.5, 0.5, 0.5),
rigid_props=sim_utils.RigidBodyPropertiesCfg(
disable_gravity=False,
),
collision_props=sim_utils.CollisionPropertiesCfg(
collision_enabled=True,
),
),
)
# sensor
sensor = ContactSensorCfg(
prim_path="/World/Robot",
)
# extras - light
light = AssetBaseCfg(
prim_path="/World/light",
spawn=sim_utils.DistantLightCfg(),
)
class TestInteractiveScene(unittest.TestCase):
"""Test cases for InteractiveScene."""
def setUp(self) -> None:
self.devices = ["cuda:0", "cpu"]
self.sim_dt = 0.001
self.scene_cfg = MySceneCfg(num_envs=1, env_spacing=1)
def test_scene_entity_isolation(self):
"""Tests that multiple instances of InteractiveScene does not share any data.
In this test, two InteractiveScene instances are created in a loop and added to a list.
The scene at index 0 of the list will have all of its entities cleared manually, and
the test compares that the data held in the scene at index 1 remained intact.
"""
for device in self.devices:
scene_list = []
# create two InteractiveScene instances
for _ in range(2):
with build_simulation_context(device=device, dt=self.sim_dt) as _:
scene = InteractiveScene(MySceneCfg(num_envs=1, env_spacing=1))
scene_list.append(scene)
scene_0 = scene_list[0]
scene_1 = scene_list[1]
# clear entities for scene_0 - this should not affect any data in scene_1
scene_0.articulations.clear()
scene_0.rigid_objects.clear()
scene_0.sensors.clear()
scene_0.extras.clear()
# check that scene_0 and scene_1 do not share entity data via dictionary comparison
self.assertEqual(scene_0.articulations, dict())
self.assertNotEqual(scene_0.articulations, scene_1.articulations)
self.assertEqual(scene_0.rigid_objects, dict())
self.assertNotEqual(scene_0.rigid_objects, scene_1.rigid_objects)
self.assertEqual(scene_0.sensors, dict())
self.assertNotEqual(scene_0.sensors, scene_1.sensors)
self.assertEqual(scene_0.extras, dict())
self.assertNotEqual(scene_0.extras, scene_1.extras)
if __name__ == "__main__":
# run main
unittest.main(verbosity=2, exit=False)
# close sim app
simulation_app.close()
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment