Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adds example on Hydra integration for running environments #67

Closed
wants to merge 7 commits into from
Closed
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@

# Outputs
**/output/*
**/outputs/*
*.tmp

# Isaac-Sim packman
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@

import torch
from dataclasses import MISSING
from typing import Optional, Sequence, Tuple, Union
from typing import Optional, List, Tuple, Union

from omni.isaac.orbit.utils import configclass
from omni.isaac.orbit.utils.math import apply_delta_pose, compute_pose_error
Expand All @@ -15,7 +15,7 @@
class OperationSpaceControllerCfg:
"""Configuration for operation-space controller."""

command_types: Sequence[str] = MISSING
command_types: List[str] = MISSING
"""Type of command.

It has two sub-strings joined by underscore:
Expand All @@ -29,9 +29,9 @@ class OperationSpaceControllerCfg:
uncouple_motion_wrench: bool = False
"""Whether to decouple the wrench computation from task-space pose (motion) error."""

motion_control_axes: Sequence[int] = (1, 1, 1, 1, 1, 1)
motion_control_axes: List[int] = (1, 1, 1, 1, 1, 1)
"""Motion direction to control. Mark as 0/1 for each axis."""
force_control_axes: Sequence[int] = (0, 0, 0, 0, 0, 0)
force_control_axes: List[int] = (0, 0, 0, 0, 0, 0)
"""Force direction to control. Mark as 0/1 for each axis."""

inertial_compensation: bool = False
Expand All @@ -40,10 +40,10 @@ class OperationSpaceControllerCfg:
gravity_compensation: bool = False
"""Whether to perform gravity compensation."""

stiffness: Union[float, Sequence[float]] = MISSING
stiffness: Union[float, List[float]] = MISSING
"""The positional gain for determining wrenches based on task-space pose error."""

damping_ratio: Optional[Union[float, Sequence[float]]] = None
damping_ratio: Optional[Union[float, List[float]]] = None
"""The damping ratio is used in-conjunction with positional gain to compute wrenches
based on task-space velocity error.

Expand All @@ -63,7 +63,7 @@ class OperationSpaceControllerCfg:
Note: Used only when :obj:`impedance_mode` is "variable".
"""

force_stiffness: Union[float, Sequence[float]] = None
force_stiffness: Union[float, List[float]] = None
"""The positional gain for determining wrenches for closed-loop force control.

If obj:`None`, then open-loop control of desired forces is performed.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
# SPDX-License-Identifier: BSD-3-Clause

from dataclasses import MISSING
from typing import Dict, Optional, Sequence, Tuple
from typing import Dict, Optional, List, Tuple

from omni.isaac.orbit.utils import configclass

Expand All @@ -21,7 +21,7 @@ class MetaInfoCfg:
"""USD file to spawn asset from."""
scale: Tuple[float] = (1.0, 1.0, 1.0)
"""Scale of the object. Default to (1.0, 1.0, 1.0)."""
sites_names: Optional[Sequence[str]] = None
sites_names: Optional[List[str]] = None
"""Name of the sites to track (added to :obj:`data`). Defaults to :obj:`None`."""

@configclass
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
# SPDX-License-Identifier: BSD-3-Clause

from dataclasses import MISSING
from typing import Optional, Sequence, Tuple
from typing import Optional, List, Tuple

from omni.isaac.orbit.utils import configclass

Expand All @@ -23,7 +23,7 @@ class MetaInfoCfg(RobotBaseCfg.MetaInfoCfg):
"""Number of degrees of freedom of arm."""
tool_num_dof: int = MISSING
"""Number of degrees of freedom of tool."""
tool_sites_names: Optional[Sequence[str]] = None
tool_sites_names: Optional[List[str]] = None
"""Name of the tool sites to track (added to :obj:`data`). Defaults to :obj:`None`.

If :obj:`None`, then no tool sites are tracked. The returned tensor for tool sites state
Expand All @@ -41,6 +41,7 @@ class EndEffectorFrameCfg:
rot_offset: Tuple[float, float, float] = (1.0, 0.0, 0.0, 0.0)
"""Additional rotation offset ``(w, x, y, z)`` from the body frame. Defaults to (1, 0, 0, 0)."""

@configclass
class DataInfoCfg:
"""Information about what all data to read from simulator.

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,9 @@
import os

# submodules
from .parse_cfg import load_default_env_cfg, parse_env_cfg
from .parse_cfg import load_default_env_cfg, parse_env_cfg, register_cfg_to_hydra

__all__ = ["load_default_env_cfg", "parse_env_cfg", "get_checkpoint_path"]
__all__ = ["load_default_env_cfg", "parse_env_cfg", "register_cfg_to_hydra", "get_checkpoint_path"]


def get_checkpoint_path(log_path: str, run_dir: str = "*", checkpoint: str = None) -> str:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
import os
import yaml
from typing import Any, Union
from hydra.core.config_store import ConfigStore

from omni.isaac.orbit.utils import update_class_from_dict, update_dict

Expand Down Expand Up @@ -111,3 +112,32 @@ def parse_env_cfg(task_name: str, use_gpu: bool = True, num_envs: int = None, **
print("[INFO]: Simulation device : ", "GPU" if args_cfg["sim"]["physx"]["use_gpu"] else "CPU")

return cfg


def register_cfg_to_hydra(task_name: str):
"""Register environment configuration to Hydra configuration store.

This function resolves the configuration file for environment based on its name.
It then registers the configuration to Hydra configuration store.

Args:
task_name (str): The name of the environment.
"""
# retrieve the configuration file to load
cfg_entry_point: str = gym.spec(task_name)._kwargs.pop("cfg_entry_point")

# parse the default config file
if callable(cfg_entry_point):
# load the configuration
cfg_cls = cfg_entry_point()
else:
# resolve path to the module location
mod_name, attr_name = cfg_entry_point.split(":")
mod = importlib.import_module(mod_name)
cfg_cls = getattr(mod, attr_name)
# load the configuration
print(f"[INFO]: Parsing default environment configuration from: {inspect.getfile(cfg_cls)}")

# register as hydra config
cs = ConfigStore.instance()
cs.store(name=task_name, node=cfg_cls)
2 changes: 2 additions & 0 deletions source/extensions/omni.isaac.orbit_envs/setup.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,8 @@
"importlib-metadata~=4.13.0",
# data collection
"h5py",
# config management
"hydra-core",
]

# Extra dependencies for RL agents
Expand Down
32 changes: 24 additions & 8 deletions source/standalone/environments/zero_agent.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,17 +8,19 @@
"""Launch Isaac Sim Simulator first."""


import sys
import argparse

from omni.isaac.kit import SimulationApp

# add argparse arguments
parser = argparse.ArgumentParser("Welcome to Orbit: Omniverse Robotics Environments!")
parser.add_argument("--headless", action="store_true", default=False, help="Force display off at all times.")
parser.add_argument("--cpu", action="store_true", default=False, help="Use CPU pipeline.")
parser.add_argument("--num_envs", type=int, default=None, help="Number of environments to simulate.")
parser.add_argument("--task", type=str, default=None, help="Name of the task.")
args_cli = parser.parse_args()
args_cli, remaining = parser.parse_known_args()
# clear out sys.argv
sys.argv = [sys.argv[0]] + remaining
sys.argc = len(sys.argv)

# launch the simulator
config = {"headless": args_cli.headless}
Expand All @@ -30,15 +32,27 @@
import gym
import torch

import hydra
from omegaconf import OmegaConf

import omni.isaac.contrib_envs # noqa: F401
import omni.isaac.orbit_envs # noqa: F401
from omni.isaac.orbit_envs.utils import parse_env_cfg
from omni.isaac.orbit_envs.isaac_env_cfg import IsaacEnvCfg
from omni.isaac.orbit_envs.utils.parse_cfg import register_cfg_to_hydra


def main():
@hydra.main(config_path=None, config_name=args_cli.task, version_base="1.3")
def main(env_cfg: IsaacEnvCfg):
"""Zero actions agent with Isaac Orbit environment."""
# parse configuration
env_cfg = parse_env_cfg(args_cli.task, use_gpu=not args_cli.cpu, num_envs=args_cli.num_envs)
# get underlying object
env_cfg = OmegaConf.to_object(env_cfg)
# modify the environment configuration
if env_cfg.sim.device == "cpu":
env_cfg.sim.use_gpu_pipeline = False
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this could be done with a pretty simple resolver directly in the config like:

OmegaConf.register_new_resolver("evaluate_use_gpu", lambda device: device.startswith('cuda'))

@dataclass
class SimConfig:
    device: str = "${sim_device}"
    ...
    use_gpu_pipeline: bool = "${evaluate_use_gpu: ${task.sim.device}}"

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh that's great! I am not an expert in Hydra so any tips are welcomed!

I was planning to have a version of train.py scripts with hydra for all the workflows too since it is quite convinient. I think to handle the agent configs and env configs together, I would need to do some sort of config composing.

Regarding the suggestion, that makes sense but I was wondering if there's a way to keep Hydra only as a thin layer and not embed hydra resolvers internally.

In general, the practice we are trying to follow is that people should be able to use other config management systems too, i.e. we are not binding to a particular solution. Though I can see how this will have some trade-offs since we won't be able to exploit the tool.

Copy link
Contributor

@romesco romesco Apr 18, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I completely agree with preserving the ability to use other config management systems. In my workflow, I tend to write dataclasses which are compatible across different configuration systems (both as schema and as direct config structures).

I try to avoid too much "config logic" within the main code base to also avoid:
1.) being locked in to that particular tool
2.) contributing a higher signal-to-noise ratio where config parsing is definitely noise haha

Let me think a bit on whether there's another way to do something like this without using the OmegaConf resolver. One possible route is through a __post_init__ call.

Copy link
Contributor Author

@Mayankm96 Mayankm96 Apr 19, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Completely with you on removing this piece of "resolving" on the main script side. Probably the post_init call can be used. Optionally, we don't do either of these and just make it a documentation thing that: for cpu/gpu implementation you need to set those particular arguments in the config.

It will make life easier because, from the simulation side, we do sometimes debug by having different use_gpu (PhysX buffers) and use_gpu_pipeline (staging buffers) flags. What this means is that using "CPU" for simulation (use_gpu=False) but reading its data on GPU (use_gpu_pipeline=True). Though this is a rather niche thing that many won't need.

Do you want to start another Github issue (proposal) to discuss recommendations that you have for using Hydra? Probably easier to keep track of this discussion than having it in the PR itself :)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah that makes sense! I'll setup a github issue for tracking other hydra-related stuff!

The explicit setting + documentation is probably the right move. However, it can be handy to have "opinionated and sane" default behavior for people to quickly jump in when they don't fully understand all of the configurable options yet, that is resolved behind the scenes e.g. __post_init__.

env_cfg.sim.physx.use_gpu = False
elif "cuda" in env_cfg.sim.device:
env_cfg.sim.use_gpu_pipeline = True
env_cfg.sim.physx.use_gpu = True
# create environment
env = gym.make(args_cli.task, cfg=env_cfg, headless=args_cli.headless)

Expand All @@ -56,8 +70,10 @@ def main():

# close the simulator
env.close()
simulation_app.close()


if __name__ == "__main__":
# configure hydra configuration
register_cfg_to_hydra(args_cli.task)
# run main function
main()