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

[RL] Review RL brains and update documentation #141

Closed
sergiopaniego opened this issue Mar 20, 2021 · 9 comments · Fixed by #144
Closed

[RL] Review RL brains and update documentation #141

sergiopaniego opened this issue Mar 20, 2021 · 9 comments · Fixed by #144
Assignees

Comments

@sergiopaniego
Copy link
Member

RL brains are currently broken, so a review of that workflow is needed.

Update the documentation about that RL brains is also needed.

@sergiopaniego sergiopaniego self-assigned this Mar 20, 2021
@sergiopaniego
Copy link
Member Author

This issue comes from the previous one -> #140

@UtkarshMishra04
Copy link
Member

Hi

So we will derive the RL brain from one of the DL brain base classes, right? If not, what approach do you plan?
Btw, I have a continuous RL brain here and I guess it is sufficient if this works

@sergiopaniego
Copy link
Member Author

I guess the idea is to review the RL part and make it work again so we shouldn't use any no RL based brain for this task. We don't need to generate a new brain now but just fix the ones already in the project (RL part).

Thanks for the brain, we can review and add it later on but this task has the maximum priority now. Without it fixed, we can't merge your brain.

@UtkarshMishra04
Copy link
Member

Hi Sergio

I have some doubts.

  1. this line closes gazebo when we try to go through an f1rl brain. Is that so?
  2. No brains are assigned when robot type is f1rl. That is not what we want. I am confused. Is the RL pipeline different from the DL pipeline?

@sergiopaniego
Copy link
Member Author

It actually is different. The RL pipeline is based on gym-gazebo, a combination of OpenAI Gym and Gazebo that has everything related to RL inside (if you follow the calls that's where the workflow is heading after that line you comment).
Additionally, you need to install some dependencies for working with RL (search for Reinforcement learning in the installation web) and the .yml is different (search for reinforcement learning in the quick start web)

There is a folder with gym-gazebo in the project root and it comes from this repo and is now under development by our org here. It will be a separated project soon by for now it's integrated in here.

The DL pipeline doesn't need the OpenAI gym part.

@UtkarshMishra04
Copy link
Member

Hi

Yes, I followed everything and I verified that whatever is required already exists in the same form as @dcharrezt formulated in his latest commit on the RL setup.
I am not sure but do you believe it is a gym-gazebo issue? Your suggestion on this will narrow down my debugging as I don't see any significant change in the RL brains after I followed all the commits of @dcharrezt, which could have caused this issue.

Thanks

@sergiopaniego
Copy link
Member Author

I can't assure that the problem comes from gym-gazebo but I believe that it's related with Behavior Studio itself. You could go back to the version of Behavior Studio from last @dcharrezt commit (somewhere in August) and verify that that version is also broken

@UtkarshMishra04
Copy link
Member

Thank you very much!

@sergiopaniego
Copy link
Member Author

I solved the problem with the RL brains. It was related with some circuits path changes and the motors publisher. Now you should be able to run both DL and RL based brains.

For the DL brains, use default.yml file and for the RL ones I've created default-rl.yml.
The steps to run the default RL training are:

  • bash load-env.sh under gym-gazebo folder.
  • cd to BehaviorStudio/behavior-studio
  • Try with the default configuration to show that it works -> python driver.py -c default-rl.yml -g
  • Change parameters/algorithm for your own training 😄

Please try it out and tell me if that worked for you.

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 a pull request may close this issue.

2 participants