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.

Sign up for GitHub

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

Jump to bottom

Adds example on Hydra integration for running environments #67

Closed
wants to merge 7 commits into from
Closed

Adds example on Hydra integration for running environments #67

Changes from 1 commit
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
a34ddf3
fixes gitignore for outputs
Mayankm96 Apr 12, 2023
959b76f
changes Sequence to List in all configclasses
Mayankm96 Apr 12, 2023
eb624c1
adds registration of configclass to hydra
Mayankm96 Apr 12, 2023
3393258
adapts zero agent for hydra
Mayankm96 Apr 12, 2023
f70290e
runs formatter over the code
Mayankm96 Apr 12, 2023
44a50cc
updates sorting of hydra and omegaconf in imports
Mayankm96 Apr 12, 2023
e143343
Merge branch 'main' into feature/hydra
Mayankm96 Apr 17, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
32 changes: 24 additions & 8 deletions source/standalone/environments/zero_agent.py
View file
Open in desktop
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 •
edited

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 •
edited

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()