Adds changes for rsl_rl 3.0.0

[ClemensSchwarke]

Description

This PR adds the necessary changes to work with the new version of RSL RL.

Type of change

• New feature (non-breaking change which adds functionality)

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

[kellyguo11]

Thanks for the changes, this is awesome! Looks like it could have some breaking changes for existing cfgs, should we include a check for the RSL RL version to force a version update if users have older versions installed?

[ClemensSchwarke]

Thanks for the changes, this is awesome! Looks like it could have some breaking changes for existing cfgs, should we include a check for the RSL RL version to force a version update if users have older versions installed?

Ah yes that would make a lot of sense I think. We could alternatively just keep all the configs unmodified as the new rsl rl version is backwards compatible, but I thought it would be nicer to show the updated way of handling normalization in the example configs. Any opinions on this?

[kellyguo11]

Thanks for the changes, this is awesome! Looks like it could have some breaking changes for existing cfgs, should we include a check for the RSL RL version to force a version update if users have older versions installed?

Ah yes that would make a lot of sense I think. We could alternatively just keep all the configs unmodified as the new rsl rl version is backwards compatible, but I thought it would be nicer to show the updated way of handling normalization in the example configs. Any opinions on this?

I think showing the new features would be nice as well. @Mayankm96 @ooctipus @pascal-roth @jtigue-bdai what are your thoughts?

[pascal-roth]

Thanks for pushing that out.

I agree we should update to the new normalisation definition but currently it is not backward compatible, should have that fixed

[pascal-roth]

It is not backwards compatible here, the key is missing in the new confit class (the key is missing in the config)

[ClemensSchwarke]

Thanks for pushing that out.

I agree we should update to the new normalisation definition but currently it is not backward compatible, should have that fixed

@pascal-roth So you are saying forcing the new rsl rl version is not the way to go?

[pascal-roth]

Thanks for pushing that out.

I agree we should update to the new normalisation definition but currently it is not backward compatible, should have that fixed

@pascal-roth So you are saying forcing the new rsl rl version is not the way to go?

No, we should update.

But in the RunnerCfg, you still need to have an attribute called: Empiricial Normalization, which throws a depreciation warning and sets the new actor and critic normalisation

[pascal-roth]

LGTM

FYI for other reviewers, checked RSL-RL and depreciation warning is thrown there https://github.com/leggedrobotics/rsl_rl/blob/main/rsl_rl/runners/on_policy_runner.py#L406

[kellyguo11]

@ClemensSchwarke looks like we might be hitting some package dependency errors in CI, are these expected?

#14 171.1 ERROR: Cannot install None, isaaclab-rl[all]==0.1.8 and isaaclab_rl because these package versions have conflicting dependencies.
#14 171.1 
#14 171.1 The conflict is caused by:
#14 171.1     isaaclab-rl 0.1.8 depends on torch==2.5.1
#14 171.1     isaaclab-rl[all] 0.1.8 depends on torch==2.5.1
#14 171.1     rsl-rl-lib 3.0.0 depends on torch>=2.6.0
#14 171.1 
#14 171.1 To fix this you could try to:
#14 171.1 1. loosen the range of package versions you've specified
#14 171.1 2. remove package versions to allow pip attempt to solve the dependency conflict
#14 171.1 
#14 171.1 ERROR: ResolutionImpossible: for help visit https://pip.pypa.io/en/latest/topics/dependency-resolution/#dealing-with-dependency-conflicts

[KingsleyLiu-NV]

Hi @ClemensSchwarke @kellyguo11, does rsl-rl 3.0 support convolution networks? If not, it still requires some customization and two PRs (one for isaaclab and one for rsl-rl) to enable the tiled-camera navigation example in PR 2027.

[ClemensSchwarke]

@ClemensSchwarke looks like we might be hitting some package dependency errors in CI, are these expected?

#14 171.1 ERROR: Cannot install None, isaaclab-rl[all]==0.1.8 and isaaclab_rl because these package versions have conflicting dependencies.
#14 171.1 
#14 171.1 The conflict is caused by:
#14 171.1     isaaclab-rl 0.1.8 depends on torch==2.5.1
#14 171.1     isaaclab-rl[all] 0.1.8 depends on torch==2.5.1
#14 171.1     rsl-rl-lib 3.0.0 depends on torch>=2.6.0
#14 171.1 
#14 171.1 To fix this you could try to:
#14 171.1 1. loosen the range of package versions you've specified
#14 171.1 2. remove package versions to allow pip attempt to solve the dependency conflict
#14 171.1 
#14 171.1 ERROR: ResolutionImpossible: for help visit https://pip.pypa.io/en/latest/topics/dependency-resolution/#dealing-with-dependency-conflicts

@kellyguo11 I think we need 2.6 for tensordict. Do you think it is feasible to switch to torch 2.6 for isaac lab?

[kellyguo11]

Yes, maybe we should cherry pick the changes we had internally to update torch to 2.7.0. The issue with only updating it in setup.py is that on windows, it'll end up installing the CPU build, which doesn't work for most of our workflows. I can put together a PR for main to update the torch installation.

@ClemensSchwarke could you also help comment on the CNN question from @KingsleyLiu-NV? Are we ok with accepting his PR for the RSL RL repo?

[ClemensSchwarke]

@kellyguo11 Ah I see, thanks a lot! I answered on slack, I think the CNN topic doesn't relate to this PR too much.

[kellyguo11]

@ClemensSchwarke could you take a look at test_rsl_rl_wrapper.py to see why it's timing out?

[ooctipus]

I have tested locally myself, looks pretty good!

[ooctipus]

I looked into the test a bit,

the test test_random_actions fails because it seems like wrapper has removed the reset. While this doesn't introduce any error in pure rsl-rl training pipeline this might break some user's workflow if they customized older rsl_rl and depended on the reset to construct their own environment.

The test is relying on obs, extras = wrapper.reset to test both observation and extra.
But in newer wrapper code, you can't get observation and extra at the same time in any alternative without stepping physics. while you can get observation from wrapper.get_observation, extra is only accessible by calling wrapper.step. (Maybe we should call step rather than reset during the test?, or maybe the test doesn't reflect the new design philosophy and should be rewrite.)

I am unsure what the proper fix should be, probably rsl team has better idea, hope this provides some quick preview of source of test failure :))

[ClemensSchwarke]

Thanks a lot for checking @ooctipus!!! I think we shouldn't keep the reset function just for the case that someone used it in a custom environment, given that older rsl_rl versions will not work with the new play/train scripts in general. Thus, someone with customized rsl_rl code would need to port that or use older lab versions anyways. However, if people think the reset function is important and can be useful, we can also add it again.

[ooctipus]

I think keeping reset or not keeping reset all make sense. If we do not keep reset, then maybe can you help on fixing the test case so it is testing the new rsl-rl without reset instead testing the old one : ))

[ClemensSchwarke]

I added the reset function again to the wrapper but not to rsl_rl, this way rsl_rl stays cleaner but the functionality is there. I also fixed the test to work with TensorDict. Let me know if that makes sense :)

[KingsleyLiu-NV]

Hi @ClemensSchwarke @kellyguo11 , since tensordict as input is a key feature of rsl-rl 3.0, why the rsl-rl exporter does not have a conditional branch handling that: exporter.py?

[ClemensSchwarke]

Hi @KingsleyLiu-NV, I am not sure if I understand you correctly. The exporter works for the current modules as it uses the dimension of the first layer of the network to infer the observation size. If you implement a different architecture, e.g., a CNN for images, the current exporter would not work. As we cannot cover all possible architectures users implement, the exporter needs to be adapted by the users in that case as well. However, we are thinking about moving the exporter inside rsl_rl to the corresponding modules. That would make it more clear that implementing a new module also necessitates implementing the corresponding export function. Does this answer your question?