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

Add wrappers for TorchRL training workflow #1178

Open
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

fyu-bdai
Copy link
Contributor

@fyu-bdai fyu-bdai commented Oct 8, 2024

Description

Adds TorchRL module wrappers, PPO runner, and PPO runner cfg for training IsaacLab environments with TorchRL.

This PR is the first in a series of three that together, adds a complete training pipeline for the Anymal-D training environment using torchrl. This PR contains core wrapper modules that should be merged first.

Related PRs:
#1179 Adds torchrl play and train scripts.
#1180 Adds Anymal-D torchrl training configuration.

Fixes #1181

⚠️ Not Supported

  • Empirical normalization.
  • Recurrent networks.
  • Training with Neptune logger.

🤔 Issues

  • Training with num_envs == 1 doesn't work.

Type of change

  • New feature (non-breaking change which adds functionality)
  • This change requires a documentation update

Checklist

  • I have run the pre-commit checks with ./isaaclab.sh --format
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • I have updated the changelog and the corresponding version in the extension's config/extension.toml file
  • I have added my name to the CONTRIBUTORS.md or my name already exists there

@fyu-bdai
Copy link
Contributor Author

fyu-bdai commented Oct 8, 2024

Can Vincent Moens also get tagged as a reviewer?

@fyu-bdai fyu-bdai changed the title Add Wrappers for TorchRL training workflow Add wrappers for TorchRL training workflow Oct 8, 2024
@fyu-bdai fyu-bdai mentioned this pull request Oct 8, 2024
6 tasks
@Toni-SM
Copy link
Contributor

Toni-SM commented Oct 8, 2024

In my opinion, non-wrapper code (e.g.: torchrl runner code) should be in the source/standalone/workflows folder.

source/standalone/workflows
├── torchrl
│   ├── ppo/      <-- code here
│   ├── cli_args.py
│   ├── play.py
│   └── train.py

Having the PPO runner code in extension will generate non-Isaac Lab related version changes and changelog records when the code need to be modified.

@fyu-bdai
Copy link
Contributor Author

fyu-bdai commented Oct 8, 2024

In my opinion, non-wrapper code (e.g.: torchrl runner code) should be in the source/standalone/workflows folder.

source/standalone/workflows
├── torchrl
│   ├── ppo/      <-- code here
│   ├── cli_args.py
│   ├── play.py
│   └── train.py

Having the PPO runner code in extension will generate non-Isaac Lab related version changes and changelog records when the code need to be modified.

I realized this causes issues with the environment specific PPO configurations, which needs to import the TorchRL PPO runner cfgs. I can move only the configs to omni.isaac.lab_tasks.utils.wrappers, but that would split the location of the torchrl runner and torchrl runner cfg. I have elected to move them back as they are for now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Proposal] Add TorchRL to IsaacLab workflows
2 participants