-
Notifications
You must be signed in to change notification settings - Fork 9
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
Slime env compatible with SB3 #5
Slime env compatible with SB3 #5
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is great, thanks!
However, I'd like to ask for an addition as a further "guarantee" of compliance: would you mind adding a test script similar to SA_QLearning.py where any single agent learning algorithm of your choice from stable-baselines3 is tested?
This way we are pretty sure that "everything works".
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe function observation_to_int_map
in SA_QLearning.py
is not necessary anymore with the new observation space?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You removed the additional boolean I put in observations with comment DOC Gym v26 has additional....
. Is it because they changed it again or beacause stable-baselines3 is not ready for Gymnasium?
In the latter case I think we should rely on this PR and not in the PIP version of stable-baselines3, so that our code will be ready when the next version of stable-baselines3 hits.
Instructions on how to use it are in the PR itself.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've seen you removed type hints, is there a specific reason?
Sure, I will do that.
I think we need it because the mapped integer is being used as a key for the custom implemented Q-learning algo right? But, do you mean for the SB3's Q Learning?
Yes, SB3 supports only gym v21 for now, not v26. I didn't know about that PR, I will look into it.
Initially, there were resulting in some error, so I tried without them. I will restore them now. |
You're right, let's leave it there for the moment, I will check if custom Q-learning implementation can be updated according to new observation space to get rid of it Thanks for all your work :) |
No problem. I had been a little occupied past week. I will push the requested changes soon. |
I have added the test training code in env-test-gym.py and its working fine.
I have restored the hints. @smarianimore please let me know in case of any more suggestions and the next steps for this project. |
Updated Slime environment that is compatible with Stable Baselines3. This PR has added/modified: