Splits terrain importer and terrain generator configuration files (#100)

# Description

Earlier the `TerrainGeneratorCfg` and `TerrainImporterCfg` were in the
same file. However, this leads to some issues with circular dependencies
when referring to the `TerrainImporter` as an attribute of the
`TerrainImporterCfg` (i.e. providing the class name as a member of the
config object).

The MR fixes the above circular dependency. Also, it moves all the
terrain parameters to its configuration object to make the terrain
initialization consistent with the other asset constructors.

## Type of change

- Breaking change (fix or feature that would cause existing
functionality to not work as expected)

## Checklist

- [x] I have run the [`pre-commit` checks](https://pre-commit.com/) with
`./orbit.sh --format`
- [ ] 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 updated the changelog and the corresponding version in the
extension's `config/extension.toml` file

source/extensions/omni.isaac.orbit/config/extension.toml

 [package]
 
 # Note: Semantic Versioning is used: https://semver.org/
-version = "0.8.1"
+version = "0.8.2"
 
 # Description
 title = "ORBIT framework for Robot Learning"

source/extensions/omni.isaac.orbit/docs/CHANGELOG.rst

 Changelog
 ---------
 
+0.8.2 (2023-08-02)
+~~~~~~~~~~~~~~~~~~
+
+Changed
+^^^^^^^
+
+* Cleaned up the :class:`omni.isaac.orbit.terrain.TerrainImporter` class to take all the parameters from the configuration
+  object. This makes it consistent with the other classes in the package.
+* Moved the configuration classes for terrain generator and terrain importer into separate files to resolve circular
+  dependency issues.
+
+
 0.8.1 (2023-08-02)
 ~~~~~~~~~~~~~~~~~~
 

source/extensions/omni.isaac.orbit/omni/isaac/orbit/terrains/__init__.py

@@ -23,8 +23,9 @@ There are two main components in this module:
 """
 
 from .height_field import *  # noqa: F401, F403
-from .terrain_cfg import SubTerrainBaseCfg, TerrainGeneratorCfg, TerrainImporterCfg
 from .terrain_generator import TerrainGenerator
+from .terrain_generator_cfg import SubTerrainBaseCfg, TerrainGeneratorCfg
 from .terrain_importer import TerrainImporter
+from .terrain_importer_cfg import TerrainImporterCfg
 from .trimesh import *  # noqa: F401, F403
 from .utils import color_meshes_by_height, create_prim_from_mesh

source/extensions/omni.isaac.orbit/omni/isaac/orbit/terrains/config/rough.py

@@ -7,7 +7,7 @@
 
 import omni.isaac.orbit.terrains as terrain_gen
 
-from ..terrain_cfg import TerrainGeneratorCfg
+from ..terrain_generator_cfg import TerrainGeneratorCfg
 
 ROUGH_TERRAINS_CFG = TerrainGeneratorCfg(
     size=(8.0, 8.0),

source/extensions/omni.isaac.orbit/omni/isaac/orbit/terrains/height_field/hf_terrains.py

@@ -15,7 +15,7 @@ from typing import TYPE_CHECKING
 from .utils import height_field_to_mesh
 
 if TYPE_CHECKING:
-    import omni.isaac.orbit.terrains.height_field.hf_terrains_cfg as hf_terrains_cfg
+    from . import hf_terrains_cfg
 
 
 @height_field_to_mesh

source/extensions/omni.isaac.orbit/omni/isaac/orbit/terrains/height_field/hf_terrains_cfg.py

@@ -6,10 +6,11 @@
 from dataclasses import MISSING
 from typing import Optional, Tuple
 
-import omni.isaac.orbit.terrains.height_field.hf_terrains as hf_terrains
-from omni.isaac.orbit.terrains.terrain_cfg import SubTerrainBaseCfg
 from omni.isaac.orbit.utils import configclass
 
+from ..terrain_generator_cfg import SubTerrainBaseCfg
+from . import hf_terrains
+
 
 @configclass
 class HfTerrainBaseCfg(SubTerrainBaseCfg):

source/extensions/omni.isaac.orbit/omni/isaac/orbit/terrains/terrain_generator.py

@@ -14,7 +14,7 @@ from omni.isaac.orbit.utils.io import dump_yaml
 from omni.isaac.orbit.utils.timer import Timer
 
 from .height_field import HfTerrainBaseCfg
-from .terrain_cfg import SubTerrainBaseCfg, TerrainGeneratorCfg
+from .terrain_generator_cfg import SubTerrainBaseCfg, TerrainGeneratorCfg
 from .trimesh.utils import make_border
 
 

source/extensions/omni.isaac.orbit/omni/isaac/orbit/terrains/terrain_cfg.py → source/extensions/omni.isaac.orbit/omni/isaac/orbit/terrains/terrain_generator_cfg.py

@@ -18,7 +18,6 @@ import numpy as np
 import trimesh
 from dataclasses import MISSING
 from typing import Callable
-from typing_extensions import Literal
 
 from omni.isaac.orbit.utils import configclass
 
@@ -124,74 +123,3 @@ class TerrainGeneratorCfg:
 
     cache_dir: str = "/tmp/orbit/terrains"
     """The directory where the terrain cache is stored. Defaults to "/tmp/orbit/terrains"."""
-
-
-@configclass
-class TerrainImporterCfg:
-    """Configuration for the terrain manager."""
-
-    prim_path: str = MISSING
-    """The absolute path of the USD terrain prim.
-
-    All sub-terrains are imported relative to this prim path.
-    """
-
-    terrain_type: Literal["generator", "plane", "usd"] = "generator"
-    """The type of terrain to generate. Defaults to "generator".
-
-    Available options are "plane", "usd", and "generator".
-    """
-
-    terrain_generator: TerrainGeneratorCfg | None = None
-    """The terrain generator configuration.
-
-    Only used if ``terrain_type`` is set to "generator".
-    """
-
-    usd_path: str | None = None
-    """The path to the USD file containing the terrain.
-
-    Only used if ``terrain_type`` is set to "usd".
-    """
-
-    color: tuple[float, float, float] | None = (0.065, 0.0725, 0.080)
-    """The color of the terrain. Defaults to (0.065, 0.0725, 0.080).
-
-    If :obj:`None`, no color is applied to the prim.
-    """
-
-    static_friction: float = 1.0
-    """The static friction coefficient of the terrain. Defaults to 1.0."""
-
-    dynamic_friction: float = 1.0
-    """The dynamic friction coefficient of the terrain. Defaults to 1.0."""
-
-    restitution: float = 0.0
-    """The restitution coefficient of the terrain. Defaults to 0.0."""
-
-    improve_patch_friction: bool = False
-    """Whether to enable patch friction. Defaults to False."""
-
-    combine_mode: str = "average"
-    """Determines the way physics materials will be combined during collisions. Defaults to `average`.
-
-    Available options are `average`, `min`, `multiply`, `multiply`, and `max`.
-    """
-
-    env_spacing: float = 3.0
-    """The spacing between environment origins when defined in a grid. Defaults to 3.0.
-
-    Note:
-      This parameter is used only when no sub-terrain origins are defined.
-    """
-
-    max_init_terrain_level: int | None = None
-    """The maximum initial terrain level for defining environment origins. Defaults to None.
-
-    The terrain levels are specified by the number of rows in the grid arrangement of
-    sub-terrains. If :obj:`None`, then the initial terrain level is set to the maximum
-    terrain level available (``num_rows - 1``).
-
-    Note:
-      This parameter is used only when sub-terrain origins are defined.
-    """

source/extensions/omni.isaac.orbit/omni/isaac/orbit/terrains/terrain_importer.py

@@ -3,14 +3,16 @@
 #
 # SPDX-License-Identifier: BSD-3-Clause
 
+from __future__ import annotations
 
 import numpy as np
 import torch
 import trimesh
-from typing import Dict, Optional, Tuple
+from typing import TYPE_CHECKING
 
 import omni.isaac.core.utils.prims as prim_utils
 import warp
+from omni.isaac.core.simulation_context import SimulationContext
 from pxr import UsdGeom
 
 from omni.isaac.orbit.markers import VisualizationMarkers
@@ -18,11 +20,13 @@ from omni.isaac.orbit.markers.config import FRAME_MARKER_CFG
 from omni.isaac.orbit.utils.kit import create_ground_plane
 from omni.isaac.orbit.utils.warp import convert_to_warp_mesh
 
-from .terrain_cfg import TerrainImporterCfg
 from .terrain_generator import TerrainGenerator
 from .trimesh.utils import make_plane
 from .utils import create_prim_from_mesh
 
+if TYPE_CHECKING:
+    from .terrain_importer_cfg import TerrainImporterCfg
+
 
 class TerrainImporter:
     r"""A class to handle terrain meshes and import them into the simulator.
@@ -41,39 +45,39 @@ class TerrainImporter:
     curriculum. For example, in a game, the player starts with easy levels and progresses to harder levels.
     """
 
-    meshes: Dict[str, trimesh.Trimesh]
+    meshes: dict[str, trimesh.Trimesh]
     """A dictionary containing the names of the meshes and their keys."""
-    warp_meshes: Dict[str, warp.Mesh]
+    warp_meshes: dict[str, warp.Mesh]
     """A dictionary containing the names of the warp meshes and their keys."""
-    terrain_origins: Optional[torch.Tensor]
+    terrain_origins: torch.Tensor | None
     """The origins of the sub-terrains in the added terrain mesh. Shape is (num_rows, num_cols, 3).
 
     If :obj:`None`, then it is assumed no sub-terrains exist. The environment origins are computed in a grid.
     """
     env_origins: torch.Tensor
-    """The origins of the environment instances. Shape is (num_envs, 3)."""
+    """The origins of the environments. Shape is (num_envs, 3)."""
 
-    def __init__(self, cfg: TerrainImporterCfg, num_envs: int, device: str):
+    def __init__(self, cfg: TerrainImporterCfg):
         """Initialize the terrain importer.
 
         Args:
             cfg (TerrainImporterCfg): The configuration for the terrain importer.
-            num_envs (int): The number of environment origins to configure.
-            device (str, optional): The device to use.
 
         Raises:
             ValueError: If input terrain type is not supported.
             ValueError: If terrain type is 'generator' and no configuration provided for ``terrain_generator``.
             ValueError: If terrain type is 'usd' and no configuration provided for ``usd_path``.
+            ValueError: If terrain type is 'usd' or 'plane' and no configuration provided for ``env_spacing``.
         """
         # store inputs
         self.cfg = cfg
-        self.device = device
+        self.device = SimulationContext.instance().device
 
         # create a dict of meshes
         self.meshes = dict()
         self.warp_meshes = dict()
         self.env_origins = None
+        self.terrain_origins = None
         # marker for visualization
         self.origin_visualizer = None
 
@@ -83,35 +87,42 @@ class TerrainImporter:
                 raise ValueError("Input terrain type is 'generator' but no value provided for 'terrain_generator'.")
             # generate the terrain
             terrain_generator = TerrainGenerator(cfg=self.cfg.terrain_generator)
-            self.import_mesh(terrain_generator.terrain_mesh, key="terrain")
+            self.import_mesh("terrain", terrain_generator.terrain_mesh)
             # configure the terrain origins based on the terrain generator
-            self.configure_env_origins(num_envs, terrain_generator.terrain_origins)
+            self.configure_env_origins(terrain_generator.terrain_origins)
         elif self.cfg.terrain_type == "usd":
             # check if config is provided
             if self.cfg.usd_path is None:
                 raise ValueError("Input terrain type is 'usd' but no value provided for 'usd_path'.")
             # import the terrain
-            self.import_usd(self.cfg.usd_path, key="terrain")
+            self.import_usd("terrain", self.cfg.usd_path)
             # configure the origins in a grid
-            self.configure_env_origins(num_envs)
+            self.configure_env_origins()
         elif self.cfg.terrain_type == "plane":
             # load the plane
-            self.import_ground_plane(key="terrain")
+            self.import_ground_plane("terrain")
             # configure the origins in a grid
-            self.configure_env_origins(num_envs)
+            self.configure_env_origins()
         else:
             raise ValueError(f"Terrain type '{self.cfg.terrain_type}' not available.")
 
-    def import_ground_plane(self, size: Tuple[int, int] = (2.0e6, 2.0e6), key: str = "terrain", **kwargs):
+    """
+    Operations - Import.
+    """
+
+    def import_ground_plane(self, key: str, size: tuple[int, int] = (2.0e6, 2.0e6), **kwargs):
         """Add a plane to the terrain importer.
 
         Args:
+            key (str): The key to store the mesh.
             size (Tuple[int, int], optional): The size of the plane. Defaults to (2.0e6, 2.0e6).
-            key (str, optional): The key to store the mesh. Defaults to "terrain".
 
         Raises:
             ValueError: If a terrain with the same key already exists.
         """
+        # check if key exists
+        if key in self.meshes:
+            raise ValueError(f"Mesh with key {key} already exists. Existing keys: {self.meshes.keys()}.")
         # create a plane
         mesh = make_plane(size, height=0.0, center_zero=True)
         # store the mesh
@@ -134,21 +145,21 @@ class TerrainImporter:
         # import the grid mesh
         create_ground_plane(self.cfg.prim_path, **mesh_props)
 
-    def import_mesh(self, mesh: trimesh.Trimesh, key: str = "terrain", **kwargs):
+    def import_mesh(self, key: str, mesh: trimesh.Trimesh, **kwargs):
         """Import a mesh into the simulator.
 
         The mesh is imported into the simulator under the prim path ``cfg.prim_path/{key}``. The created path
         contains the mesh as a :class:`pxr.UsdGeom` instance along with visual or physics material prims.
 
         Args:
+            key (str): The key to store the mesh.
             mesh (trimesh.Trimesh): The mesh to import.
-            key (str, optional): The key to store the mesh. Defaults to "terrain".
             **kwargs: The properties of the mesh. If not provided, the default properties are used.
 
         Raises:
             ValueError: If a terrain with the same key already exists.
         """
-        # add mesh to the dict
+        # check if key exists
         if key in self.meshes:
             raise ValueError(f"Mesh with key {key} already exists. Existing keys: {self.meshes.keys()}.")
         # store the mesh
@@ -174,7 +185,7 @@ class TerrainImporter:
         # import the mesh
         create_prim_from_mesh(mesh_prim_path, mesh.vertices, mesh.faces, **mesh_props)
 
-    def import_usd(self, usd_path: str, key: str = "terrain"):
+    def import_usd(self, key: str, usd_path: str):
         """Import a mesh from a USD file.
 
         We assume that the USD file contains a single mesh. If the USD file contains multiple meshes, then
@@ -186,8 +197,8 @@ class TerrainImporter:
             be defined in the USD file.
 
         Args:
+            key (str): The key to store the mesh.
             usd_path (str): The path to the USD file.
-            key (str, optional): The key to store the mesh. Defaults to "terrain".
 
         Raises:
             ValueError: If a terrain with the same key already exists.
@@ -215,15 +226,18 @@ class TerrainImporter:
         device = "cuda" if "cuda" in self.device else "cpu"
         self.warp_meshes[key] = convert_to_warp_mesh(vertices, faces, device=device)
 
-    def configure_env_origins(self, num_envs: int, origins: Optional[np.ndarray] = None):
+    """
+    Operations - Origins.
+    """
+
+    def configure_env_origins(self, origins: np.ndarray | None = None):
         """Configure the origins of the environments based on the added terrain.
 
         Args:
-            num_envs (int): The number of environment origins to define.
             origins (Optional[np.ndarray]): The origins of the sub-terrains. Shape: (num_rows, num_cols, 3).
         """
         # create markers for the origins
-        markers = VisualizationMarkers(f"{self.cfg.prim_path}/originMarkers", cfg=FRAME_MARKER_CFG)
+        markers = VisualizationMarkers("/Visuals/TerrainOrigin", cfg=FRAME_MARKER_CFG)
         # decide whether to compute origins in a grid or based on curriculum
         if origins is not None:
             # convert to numpy
@@ -232,13 +246,16 @@ class TerrainImporter:
             # store the origins
             self.terrain_origins = origins.to(self.device, dtype=torch.float)
             # compute environment origins
-            self.env_origins = self._compute_env_origins_curriculum(num_envs, self.terrain_origins)
+            self.env_origins = self._compute_env_origins_curriculum(self.cfg.num_envs, self.terrain_origins)
             # put markers on the sub-terrain origins
             markers.visualize(self.terrain_origins.reshape(-1, 3))
         else:
             self.terrain_origins = None
+            # check if env spacing is valid
+            if self.cfg.env_spacing is None:
+                raise ValueError("Environment spacing must be specified for configuring grid-like origins.")
             # compute environment origins
-            self.env_origins = self._compute_env_origins_grid(num_envs, self.cfg.env_spacing)
+            self.env_origins = self._compute_env_origins_grid(self.cfg.num_envs, self.cfg.env_spacing)
             # put markers on the grid origins
             markers.visualize(self.env_origins.reshape(-1, 3))
 

source/extensions/omni.isaac.orbit/omni/isaac/orbit/terrains/terrain_importer_cfg.py

+# Copyright (c) 2022, NVIDIA CORPORATION & AFFILIATES, ETH Zurich, and University of Toronto
+# All rights reserved.
+#
+# SPDX-License-Identifier: BSD-3-Clause
+
+from __future__ import annotations
+
+from dataclasses import MISSING
+from typing import TYPE_CHECKING
+from typing_extensions import Literal
+
+from omni.isaac.orbit.utils import configclass
+
+from .terrain_importer import TerrainImporter
+
+if TYPE_CHECKING:
+    from .terrain_generator_cfg import TerrainGeneratorCfg
+
+
+@configclass
+class TerrainImporterCfg:
+    """Configuration for the terrain manager."""
+
+    cls_name: type = TerrainImporter
+    """The class name of the terrain importer."""
+
+    prim_path: str = MISSING
+    """The absolute path of the USD terrain prim.
+
+    All sub-terrains are imported relative to this prim path.
+    """
+
+    num_envs: int = MISSING
+    """The number of environment origins to consider."""
+
+    terrain_type: Literal["generator", "plane", "usd"] = "generator"
+    """The type of terrain to generate. Defaults to "generator".
+
+    Available options are "plane", "usd", and "generator".
+    """
+
+    terrain_generator: TerrainGeneratorCfg | None = None
+    """The terrain generator configuration.
+
+    Only used if ``terrain_type`` is set to "generator".
+    """
+
+    usd_path: str | None = None
+    """The path to the USD file containing the terrain.
+
+    Only used if ``terrain_type`` is set to "usd".
+    """
+
+    env_spacing: float | None = None
+    """The spacing between environment origins when defined in a grid. Defaults to None.
+
+    Note:
+      This parameter is used only when the ``terrain_type`` is ``"plane"`` or ``"usd"``.
+    """
+
+    color: tuple[float, float, float] | None = (0.065, 0.0725, 0.080)
+    """The color of the terrain. Defaults to (0.065, 0.0725, 0.080).
+
+    If :obj:`None`, no color is applied to the prim.
+    """
+
+    static_friction: float = 1.0
+    """The static friction coefficient of the terrain. Defaults to 1.0."""
+
+    dynamic_friction: float = 1.0
+    """The dynamic friction coefficient of the terrain. Defaults to 1.0."""
+
+    restitution: float = 0.0
+    """The restitution coefficient of the terrain. Defaults to 0.0."""
+
+    improve_patch_friction: bool = False
+    """Whether to enable patch friction. Defaults to False."""
+
+    combine_mode: str = "average"
+    """Determines the way physics materials will be combined during collisions. Defaults to `average`.
+
+    Available options are `average`, `min`, `multiply`, `multiply`, and `max`.
+    """
+
+    max_init_terrain_level: int | None = None
+    """The maximum initial terrain level for defining environment origins. Defaults to None.
+
+    The terrain levels are specified by the number of rows in the grid arrangement of
+    sub-terrains. If :obj:`None`, then the initial terrain level is set to the maximum
+    terrain level available (``num_rows - 1``).
+
+    Note:
+      This parameter is used only when sub-terrain origins are defined.
+    """

source/extensions/omni.isaac.orbit/omni/isaac/orbit/terrains/trimesh/mesh_terrains_cfg.py

@@ -8,9 +8,10 @@ from typing import Tuple, Union
 
 import omni.isaac.orbit.terrains.trimesh.mesh_terrains as mesh_terrains
 import omni.isaac.orbit.terrains.trimesh.utils as mesh_utils_terrains
-from omni.isaac.orbit.terrains.terrain_cfg import SubTerrainBaseCfg
 from omni.isaac.orbit.utils import configclass
 
+from ..terrain_generator_cfg import SubTerrainBaseCfg
+
 """
 Different trimesh terrain configurations.
 """

source/extensions/omni.isaac.orbit/test/terrains/test_terrain_importer.py

@@ -96,13 +96,15 @@ def main(terrain_type: str):
 
     # Handler for terrains importing
     terrain_importer_cfg = terrain_gen.TerrainImporterCfg(
+        num_envs=2048,
+        env_spacing=3.0,
         prim_path="/World/ground",
         max_init_terrain_level=None,
         terrain_type=terrain_type,
         terrain_generator=ROUGH_TERRAINS_CFG.replace(curriculum=True),
         usd_path=f"{ISAAC_NUCLEUS_DIR}/Environments/Terrains/rough_plane.usd",
     )
-    terrain_importer = TerrainImporter(terrain_importer_cfg, num_envs=num_balls, device=sim.device)
+    terrain_importer = TerrainImporter(terrain_importer_cfg)
 
     # Define the scene
     # -- Light

source/extensions/omni.isaac.orbit_envs/omni/isaac/orbit_envs/locomotion/velocity/locomotion_env.py

@@ -110,6 +110,8 @@ class LocomotionEnv(IsaacEnv):
 
         # terrain
         self.cfg.terrain.prim_path = "/World/ground"
+        self.cfg.terrain.num_envs = self.cfg.env.num_envs
+        self.cfg.terrain.env_spacing = self.cfg.env.env_spacing
         # check if curriculum is enabled
         if self.cfg.terrain.terrain_type == "generator":
             if hasattr(self.cfg.curriculum, "terrain_levels") and self.cfg.curriculum.terrain_levels is not None:
@@ -117,7 +119,7 @@ class LocomotionEnv(IsaacEnv):
             else:
                 self.cfg.terrain.terrain_generator.curriculum = False
         # self.cfg.terrain.max_init_terrain_level = None
-        self.terrain_importer = TerrainImporter(self.cfg.terrain, self.num_envs, device=self.device)
+        self.terrain_importer = TerrainImporter(self.cfg.terrain)
 
         # robot
         self.robot.spawn(self.template_env_ns + "/Robot")