multiple refactors (#233)

* refactoring

* bug fixes

* refactor: _register_* to _sensor_configs property

* refactoring default sim cfg

* consistency work

* consistent namings

docs/source/user_guide/tutorials/custom_tasks.md

@@ -238,21 +238,22 @@ Dense reward functions are not required and can be skipped. If not implemented t
 
 ## (Optional) Setting up Cameras/Sensors for Observations and Recording
 
-If you want your task to be able to return information from sensors like cameras as part of observations, you need to implement a `_register_sensors` function which should return a list of `SensorConfig` objects. At the moment the only sensor implemented are Cameras. In the future other forms of sensors will be added. Adding a `_register_human_render_cameras` function will add cameras to be used to take pictures for the `"rgb_array"` render mode, which is usually used just for saving videos to look at, but are never used as part of the actual environment observations.
+If you want your task to be able to return information from sensors like cameras as part of observations, you need to implement a `_sensor_configs` property which should return a list of `BaseSensorConfig` objects. At the moment the only sensor implemented are Cameras. In the future other forms of sensors will be added. Adding a `_human_render_camera_configs` property will add cameras to be used to take pictures for the `"rgb_array"` render mode, which is usually used just for saving videos to look at, but are never used as part of the actual environment observations.
 
 Below shows how to use `CameraConfig` to define sensors, you define its position, quaternion, width, height, fov, near, and far attributes. 
 
 ```python
 from mani_skill.sensors.camera import CameraConfig
-def _register_sensors(self):
+@property
+def _sensor_configs(self):
     # registers one 128x128 camera looking at the robot, cube, and target
     # a smaller sized camera will be lower quality, but render faster
     pose = sapien_utils.look_at(eye=[0.3, 0, 0.6], target=[-0.1, 0, 0.1])
     return [
         CameraConfig("base_camera", pose.p, pose.q, 128, 128, 1, 0.01, 10)
     ]
-
-def _register_human_render_cameras(self):
+@property
+def _human_render_camera_configs(self):
     # registers a more high-definition (512x512) camera used just for rendering when render_mode="rgb_array" or calling env.render_rgb_array()
     pose = sapien_utils.look_at([0.6, 0.7, 0.6], [0.0, 0.0, 0.35])
     return CameraConfig("render_camera", pose.p, pose.q, 512, 512, 1, 0.01, 10)
@@ -278,5 +279,5 @@ Alternatively via the GUI which can be opened by doing a while loop while runnin
 :::
 
 :::{tip}
-It's recommended to setup the sensor cameras via `_register_sensors` in such a way so that it looks at the important objects and avoids looking at anything too far away. The reason is the blank background has infinite depth and in visual observations it's marked as a 0. Objects too far away (like the far away floor tiles) will yield very high depth values which may be problematic for machine learning workflows
+It's recommended to setup the sensor cameras via `_sensor_configs` in such a way so that it looks at the important objects and avoids looking at anything too far away. The reason is the blank background has infinite depth and in visual observations it's marked as a 0. Objects too far away (like the far away floor tiles) will yield very high depth values which may be problematic for machine learning workflows
 :::

docs/source/user_guide/tutorials/custom_tasks_advanced.md

@@ -201,14 +201,14 @@ class SimConfig:
 ```
 :::
 
-To define a different set of default sim configurations, you can define a `default_sim_cfg` property in your task class with the SimConfig etc. dataclasses as so
+To define a different set of default sim configurations, you can define a `_default_sim_cfg` property in your task class with the SimConfig etc. dataclasses as so
 
 ```python
 from mani_skill.utils.structs.types import GPUMemoryConfig, SimConfig
 class MyCustomTask(BaseEnv)
     # ...
     @property
-    def default_sim_cfg(self):
+    def _default_sim_cfg(self):
         return SimConfig(
             gpu_memory_cfg=GPUMemoryConfig(
                 max_rigid_contact_count=self.num_envs * max(1024, self.num_envs) * 8,
@@ -218,7 +218,7 @@ class MyCustomTask(BaseEnv)
         )
 ```
 
-ManiSkill will fetch `default_sim_cfg` after `self.num_envs` is set so you can also dynamically change configurations at runtime depending on the number of environments like it was done above. You usually need to change the default configurations when you try to run more parallel environments but SAPIEN will print critical errors about needing to increase one of the GPU memory configuration options.
+ManiSkill will fetch `_default_sim_cfg` after `self.num_envs` is set so you can also dynamically change configurations at runtime depending on the number of environments like it was done above. You usually need to change the default configurations when you try to run more parallel environments but SAPIEN will print critical errors about needing to increase one of the GPU memory configuration options.
 
 Some of the other important configuration options and their defaults that are part of SimConfig are `spacing=5`, `sim_freq=100`, `control_freq=20`, and `'solver_iterations=15`. The physx timestep of the simulation is computed as `1 / sim_freq`, and the `control_freq` says that every `sim_freq/control_freq` physx steps we apply the environment action once and then fetch observation data to return to the user. 
 
@@ -241,11 +241,12 @@ Note the default `sim_freq, control_freq` values are tuned for GPU simulation an
 
 The custom tasks tutorial demonstrated adding fixed cameras to the PushCube task. ManiSkill+SAPIEN also supports mounting cameras to Actors and Links, which can be useful to e.g. have a camera follow a object as it moves around.
 
-For example if you had a task with a baseketball in it and it's actor object is stored at `self.basketball`, in the `_register_sensors` or `_register_human_render_cameras` functions you can do
+For example if you had a task with a baseketball in it and it's actor object is stored at `self.basketball`, in the `_sensor_configs` or `_human_render_camera_configs` properties you can do
 
 ```python
 
-def _register_sensors(self)
+@property
+def _sensor_configs(self)
     # look towards the center of the baskeball from a positon that is offset
     # (0.3, 0.3, 0.5) away from the basketball
     pose = sapien_utils.look_at(eye=[0.3, 0.3, 0.5], target=[0, 0, 0])

mani_skill/agents/base_agent.py

@@ -1,7 +1,7 @@
 from __future__ import annotations
 
 from collections import OrderedDict
-from typing import TYPE_CHECKING, Dict, Union
+from typing import TYPE_CHECKING, Dict, Union, List
 
 import numpy as np
 import sapien
@@ -23,7 +23,7 @@
 
 if TYPE_CHECKING:
     from mani_skill.envs.scene import ManiSkillScene
-
+DictControllerConfig = Dict[str, ControllerConfig]
 
 class BaseAgent:
     """Base class for agents.
@@ -40,17 +40,12 @@ class BaseAgent:
     """
 
     uid: str
-    robot: Articulation
-
+    """unique identifier string of this"""
     urdf_path: str
-    urdf_config: dict
-
-    controller_configs: Dict[str, Union[ControllerConfig, Dict[str, ControllerConfig]]]
-    controllers: Dict[str, BaseController]
-
-    sensor_configs: Dict[str, BaseSensorConfig]
-    sensors: Dict[str, BaseSensor]
-
+    """path to the .urdf file describe the agent's geometry and visuals"""
+    urdf_config: dict = None
+    """Optional provide a urdf_config to further modify the created articulation"""
+
     def __init__(
         self,
         scene: ManiSkillScene,
@@ -65,9 +60,13 @@ def __init__(
 
         # URDF
         self.fix_root_link = fix_root_link
+
+        self.robot: Articulation = None
+        self.controllers: Dict[str, BaseController] = dict()
+        self.sensors: Dict[str, BaseSensor] = dict()
 
         # Controller
-        self.supported_control_modes = list(self.controller_configs.keys())
+        self.supported_control_modes = list(self._controller_configs.keys())
         if control_mode is None:
             control_mode = self.supported_control_modes[0]
         # The control mode after reset for consistency
@@ -76,6 +75,15 @@ def __init__(
         self._load_articulation()
         self._after_loading_articulation()
         self._after_init()
+        self.set_control_mode()
+
+    @property
+    def _sensor_configs(self) -> List[str, BaseSensorConfig]:
+        return []
+
+    @property
+    def _controller_configs(self) -> Dict[str, Union[ControllerConfig, DictControllerConfig]]:
+        raise NotImplementedError()
 
     @property
     def device(self):
@@ -93,8 +101,9 @@ def _load_articulation(self):
 
         urdf_path = format_path(str(self.urdf_path))
 
-        urdf_config = sapien_utils.parse_urdf_config(self.urdf_config, self.scene)
-        sapien_utils.check_urdf_config(urdf_config)
+        if self.urdf_config is not None:
+            urdf_config = sapien_utils.parse_urdf_config(self.urdf_config, self.scene)
+            sapien_utils.check_urdf_config(urdf_config)
 
         # TODO(jigu): support loading multiple convex collision shapes
         sapien_utils.apply_urdf_config(loader, urdf_config)
@@ -133,7 +142,7 @@ def set_control_mode(self, control_mode=None):
         self._control_mode = control_mode
         # create controller on the fly here
         if control_mode not in self.controllers:
-            config = self.controller_configs[self._control_mode]
+            config = self._controller_configs[self._control_mode]
             if isinstance(config, dict):
                 self.controllers[control_mode] = CombinedController(
                     config, self.robot, self._control_freq, scene=self.scene

mani_skill/agents/multi_agent.py

@@ -18,7 +18,7 @@ def __init__(self, agents: List[BaseAgent]):
         self.scene = agents[0].scene
         self.sensor_configs = []
         for i, agent in enumerate(self.agents):
-            self.sensor_configs += agent.sensor_configs
+            self.sensor_configs += agent._sensor_configs
             self.agents_dict[f"{agent.uid}-{i}"] = agent
 
     def get_proprioception(self):

mani_skill/agents/robots/_template/template_robot.py

@@ -37,7 +37,7 @@ def __init__(self, *args, **kwargs):
         super().__init__(*args, **kwargs)
 
     @property
-    def controller_configs(self):
+    def _controller_configs(self):
         raise NotImplementedError()
 
     @property

mani_skill/agents/robots/allegro_hand/allegro.py

@@ -81,7 +81,7 @@ def _after_init(self):
         )
 
     @property
-    def controller_configs(self):
+    def _controller_configs(self):
         # -------------------------------------------------------------------------- #
         # Arm
         # -------------------------------------------------------------------------- #

mani_skill/agents/robots/anymal/anymal_c.py

@@ -33,7 +33,7 @@ def __init__(self, *args, **kwargs):
         super().__init__(*args, fix_root_link=False, **kwargs)
 
     @property
-    def controller_configs(self):
+    def _controller_configs(self):
         self.arm_stiffness = 85.0
         self.arm_damping = 2.0
         self.arm_force_limit = 100

mani_skill/agents/robots/dclaw/dclaw.py

@@ -63,7 +63,7 @@ def _after_init(self):
         )
 
     @property
-    def controller_configs(self):
+    def _controller_configs(self):
         # -------------------------------------------------------------------------- #
         # Arm
         # -------------------------------------------------------------------------- #

mani_skill/agents/robots/fetch/fetch.py

@@ -127,7 +127,7 @@ def __init__(self, *args, **kwargs):
         super().__init__(*args, **kwargs)
 
     @property
-    def controller_configs(self):
+    def _controller_configs(self):
         # -------------------------------------------------------------------------- #
         # Arm
         # -------------------------------------------------------------------------- #

mani_skill/agents/robots/panda/panda.py

@@ -60,7 +60,7 @@ def __init__(self, *args, **kwargs):
         super().__init__(*args, **kwargs)
 
     @property
-    def controller_configs(self):
+    def _controller_configs(self):
         # -------------------------------------------------------------------------- #
         # Arm
         # -------------------------------------------------------------------------- #
@@ -195,8 +195,6 @@ def controller_configs(self):
         # Make a deepcopy in case users modify any config
         return deepcopy_dict(controller_configs)
 
-    sensor_configs = []
-
     def _after_init(self):
         self.finger1_link = sapien_utils.get_obj_by_name(
             self.robot.get_links(), "panda_leftfinger"

mani_skill/agents/robots/xarm/xarm7_ability.py

@@ -74,7 +74,7 @@ def __init__(self, *args, **kwargs):
         super().__init__(*args, **kwargs)
 
     @property
-    def controller_configs(self):
+    def _controller_configs(self):
         # -------------------------------------------------------------------------- #
         # Arm
         # -------------------------------------------------------------------------- #
@@ -145,8 +145,6 @@ def controller_configs(self):
         # Make a deepcopy in case users modify any config
         return deepcopy_dict(controller_configs)
 
-    sensor_configs = []
-
     def _after_init(self):
         hand_front_link_names = [
             "thumb_L2",

mani_skill/agents/robots/xmate3/xmate3.py

@@ -80,7 +80,7 @@ def _after_init(self):
         ] = dict()
 
     @property
-    def controller_configs(self):
+    def _controller_configs(self):
         # -------------------------------------------------------------------------- #
         # Arm
         # -------------------------------------------------------------------------- #
@@ -152,32 +152,34 @@ def controller_configs(self):
         # Make a deepcopy in case users modify any config
         return deepcopy_dict(controller_configs)
 
-    sensor_configs = [
-        CameraConfig(
-            uid="base_camera",
-            p=[0.0, 0.0, 0.0],
-            q=[1, 0, 0, 0],
-            width=128,
-            height=128,
-            fov=1.5707,
-            near=0.01,
-            far=100,
-            entity_uid="camera_base_link",
-            hide_link=False,
-        ),
-        CameraConfig(
-            uid="hand_camera",
-            p=[0.0, 0.0, 0.0],
-            q=[1, 0, 0, 0],
-            width=128,
-            height=128,
-            fov=1.5707,
-            near=0.01,
-            far=100,
-            entity_uid="camera_hand_link",
-            hide_link=False,
-        ),
-    ]
+    @property
+    def _sensor_configs(self):
+        return [
+            CameraConfig(
+                uid="base_camera",
+                p=[0.0, 0.0, 0.0],
+                q=[1, 0, 0, 0],
+                width=128,
+                height=128,
+                fov=1.5707,
+                near=0.01,
+                far=100,
+                entity_uid="camera_base_link",
+                hide_link=False,
+            ),
+            CameraConfig(
+                uid="hand_camera",
+                p=[0.0, 0.0, 0.0],
+                q=[1, 0, 0, 0],
+                width=128,
+                height=128,
+                fov=1.5707,
+                near=0.01,
+                far=100,
+                entity_uid="camera_hand_link",
+                hide_link=False,
+            ),
+        ]
 
     def is_grasping(self, object: Actor = None, min_impulse=1e-6, max_angle=85):
         if physx.is_gpu_enabled():

mani_skill/envs/minimal_template.py

@@ -31,23 +31,24 @@ class CustomEnv(BaseEnv):
 
     SUPPORTED_ROBOTS = ["panda", "fetch"]
     agent: Union[Panda, Fetch]
-    default_sim_cfg = SimConfig(
-        gpu_memory_cfg=GPUMemoryConfig(
-            found_lost_pairs_capacity=2**25, max_rigid_patch_count=2**18
-        )
-    )
 
     def __init__(self, *args, robot_uids="panda", robot_init_qpos_noise=0.02, **kwargs):
         self.robot_init_qpos_noise = robot_init_qpos_noise
         super().__init__(*args, robot_uids=robot_uids, **kwargs)
 
-    def _register_sensors(self):
+    @property
+    def _default_sim_cfg(self):
+        return SimConfig()
+
+    @property
+    def _sensor_configs(self):
         pose = sapien_utils.look_at(eye=[0.3, 0, 0.6], target=[-0.1, 0, 0.1])
         return [
             CameraConfig("base_camera", pose.p, pose.q, 128, 128, np.pi / 2, 0.01, 100)
         ]
 
-    def _register_human_render_cameras(self):
+    @property
+    def _human_render_camera_configs(self):
         pose = sapien_utils.look_at([0.6, 0.7, 0.6], [0.0, 0.0, 0.35])
         return CameraConfig("render_camera", pose.p, pose.q, 512, 512, 1, 0.01, 100)