IsaacLab icon indicating copy to clipboard operation
IsaacLab copied to clipboard

feat: SB3 optimizations (3x faster)

Open araffin opened this issue 9 months ago • 3 comments

Description

Please include a summary of the change and which issue is fixed. Please also include relevant motivation and context. List any dependencies that are required for this change.

Implement part of https://github.com/isaac-sim/IsaacLab/issues/1769 (optimization)

This is a breaking change because the fast variant is now enabled by default.

I also improve sb3 training script, fixed loading of normalization and fixed the humanoid hyperparameters to be similar to rsl-rl, so we can compare apples to apples in terms of training speed.

I will probably open another PR for the rest of the proposals.

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update

With respect to testing, how do you run a single test? and is there anything I should add?

Checklist

  • [x] I have run the pre-commit checks with ./isaaclab.sh --format
  • [x] I have made corresponding changes to the documentation
  • [x] My changes generate no new warnings
  • [ ] 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
  • [x] I have added my name to the CONTRIBUTORS.md or my name already exists there

araffin avatar Mar 05 '25 11:03 araffin

@Mayankm96 failure on the CI seems to be due to a timeout, probably unrelated to this PR?

araffin avatar Mar 07 '25 08:03 araffin

@Mayankm96 when do you think that you or someone from your team can have a look at this PR? and do you need anything else from me?

araffin avatar Mar 22 '25 10:03 araffin

Hi @araffin , sorry for the delay. I have assigned @Toni-SM to help review this MR. Thanks a lot for the improvements introduced!

Mayankm96 avatar Mar 23 '25 11:03 Mayankm96

@Toni-SM could you please re-review this MR so we can merge it in a timely manner. Thank you!

Mayankm96 avatar May 09 '25 09:05 Mayankm96

@Mayankm96 could you maybe assign someone else? Antonio seems pretty busy those last months.

araffin avatar Jun 11 '25 06:06 araffin

Toni has reviewed the PR, we'll try to get this merged in soon. Thanks for staying on top of this, @araffin!

kellyguo11 avatar Jun 11 '25 16:06 kellyguo11

do you mind adding in the version update in extension.toml? then we can get this merged.

kellyguo11 avatar Jun 23 '25 22:06 kellyguo11