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

Bipedal Walker - Fix initial body position #782

Closed

Conversation

LiuShuai26
Copy link

Description

I found a small error in the bipedal_walker environment. When resetting the environment, it passes a wrong body position. The position mismatch causes the Box2D physics engine to resolve it, thereby affecting the initial state of the environment and the following few frames. This issue might impact the training process.

Type of change

Please delete options that are not relevant.

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

Checklist:

  • I have run the pre-commit checks with pre-commit run --all-files (see CONTRIBUTING.md instructions to set it up)
  • I have commented my code, particularly in hard-to-understand areas
  • 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
  • New and existing unit tests pass locally with my changes

@pseudo-rnd-thoughts
Copy link
Member

Thanks for the PR, could you explain the impact that the change will have?
Could you visualise the difference between the changes for the same seed?

@pseudo-rnd-thoughts pseudo-rnd-thoughts changed the title fix body initial position Bipedal Walker - Fix initial body position Nov 21, 2023
@LiuShuai26
Copy link
Author

To visualize the difference, I changed line categoryBits=0x0001, to set categoryBits as 0x0002, allowing the bipedal walker's lower leg to overlap with the terrain rather than being resolved by the physics engine. Below is the first frame. It is evident that the original code's initial position causes an overlap between the two rigid bodies (lower leg and terrain).

Before After
Screenshot from 2023-11-21 22-34-07 Screenshot from 2023-11-21 22-33-35

|

@pseudo-rnd-thoughts
Copy link
Member

Ok that makes sense, @LiuShuai26 could you do some testing to investigate the impact on training performance?
Do agent get better / worse / equal scores? Do old agents using the old initialisation get better / worse / equal scores?

@LiuShuai26
Copy link
Author

I ran three experiments for both the old and new environments with different seed. I am using sample-factory without tuning. It seems to yield identical results. A slight disruption in the first few frames appears to have insufficient impact on the training process.
Screenshot from 2023-11-26 14-59-03

@pseudo-rnd-thoughts
Copy link
Member

Thanks for running the tests, what is the training algorithm used and I'm surprised that this required 40 million observation to train

@LiuShuai26
Copy link
Author

Thanks for running the tests, what is the training algorithm used and I'm surprised that this required 40 million observation to train

I am using the Sample Factory with the APPO algorithm, without tuning, and the training parameters are set to default. There will be a data efficiency problem when training in a small environment because Sample Factory is designed for high throughput.

command line parameter from Sample Factory example:
--algo=APPO --use_rnn=False --num_envs_per_worker=20 --policy_workers_per_policy=2 --recurrence=1 --with_vtrace=False --batch_size=512 --reward_scale=0.1

@pseudo-rnd-thoughts
Copy link
Member

Could you rerun this with stable baselines 3 or cleanrl?

@LiuShuai26
Copy link
Author

I tried using stable-baselines3 and cleanrl initially, but both of them were not ready to go. Following the documentation to launch training resulted in multiple errors. Unfortunately, I don't have the time to address these issues.

@LiuShuai26 LiuShuai26 closed this Dec 8, 2023
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.

2 participants