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

Pull Request #113 breaks Issac Gym #121

Closed
Tbarkin121 opened this issue Feb 16, 2022 · 6 comments
Closed

Pull Request #113 breaks Issac Gym #121

Tbarkin121 opened this issue Feb 16, 2022 · 6 comments

Comments

@Tbarkin121
Copy link

Tbarkin121 commented Feb 16, 2022

I was modifying some of the rl_games code when I noticed newer version do not work with Issac Gym. Prior to this merge #113 things appear to be working correctly.

@Tbarkin121 Tbarkin121 changed the title Merge #113 breaks Issac Gym Pull Request #113 breaks Issac Gym Feb 16, 2022
@Denys88
Copy link
Owner

Denys88 commented Feb 17, 2022

Could you show your error please?
They idea of this pr is to move normalization to the model out of algorithm.
It worked for me, but it may not work with your changes and I am happy to help.

@Tbarkin121
Copy link
Author

It doesn't have an error but it seems like the normalization being moved effects the output actions. When loading from a checkpoint and running the environment with test=True, the resulting actions seem scaled down by a large amount.

I had a suspicion that the normalization is missing from the models Isaac Gym is using, but I wasn't 100% sure where to start to fix this issue.

@ViktorM
Copy link
Collaborator

ViktorM commented Feb 19, 2022

Hi @Tbarkin121! Can you please describe the repro steps - I've been training and testing a lot of different environments recently with the latest rl-games and they all worked fine for me.

@Denys88
Copy link
Owner

Denys88 commented Feb 22, 2022

@Tbarkin121 what if you train in master and try to test in master?
I broke backward compatibility for old checkpoints now.

@Denys88
Copy link
Owner

Denys88 commented Feb 22, 2022

@Tbarkin121 I've found the issue.
I removed load_path from the yaml config but Isaacgym never sent this parameter to me.
The fastest way is to fix it in the train.py in isaacgym:
runner.run({
'train': not cfg.test,
'play': cfg.test,
'checkpoint' : cfg.checkpoint # just add this line
})

@Denys88
Copy link
Owner

Denys88 commented Mar 18, 2022

I added a few more fixes for the backward compatibility.

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

No branches or pull requests

3 participants