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

Update sb3_kaz_vector.py #1132

Closed
wants to merge 2 commits into from

Conversation

Fernadoo
Copy link
Contributor

Description

In the tutorial of utilizing SB3 for the KAZ environment, the original author updates the reward Dict using the same variable name in an inner loop as that used for env.agent_iter(), which leads to the consequence that the actual decision-maker is always the last agent namely knight_1. I have just revised it to a different name and it works fine.

Type of change

  • Bug fix (non-breaking change which fixes an issue)

Checklist:

  • I have run the pre-commit checks with pre-commit run --all-files (see CONTRIBUTING.md instructions to set it up)
  • I have run pytest -v and no errors are present.
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • I solved any possible warnings that pytest -v has generated that are related to my code to the best of my knowledge.
  • 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

@elliottower
Copy link
Contributor

Thanks for finding this, makes sense cause I remember testing it locally and it seemed like it wasn’t learning for the other agents, figured it was due to the number of steps but this makes way more sense. Any chance you could check to see if any of the other SB3 tutorials have issues? I mainly adapted the old SB3 tutorial jordan (original creator of PZ) did on a medium article, but I may not have done everything correctly. Have had a bunch of people asking questions about it, haven’t had time to look deeply myself and am not an expert when it comes to training algorithms and such.

@elliottower
Copy link
Contributor

Looks like the other tutorials have the same error, but I can just fix it myself.

@Fernadoo Fernadoo requested a review from elliottower November 15, 2023 00:56
@Fernadoo
Copy link
Contributor Author

Thanks for finding this, makes sense cause I remember testing it locally and it seemed like it wasn’t learning for the other agents, figured it was due to the number of steps but this makes way more sense. Any chance you could check to see if any of the other SB3 tutorials have issues? I mainly adapted the old SB3 tutorial jordan (original creator of PZ) did on a medium article, but I may not have done everything correctly. Have had a bunch of people asking questions about it, haven’t had time to look deeply myself and am not an expert when it comes to training algorithms and such.

Sure thing, I'll try to help debug those tutorials once I have some spare time 🥸

@elliottower elliottower mentioned this pull request Nov 15, 2023
7 tasks
@elliottower
Copy link
Contributor

Closing this as I'm just going to fix the pre-commit and do the other tutorials as well, I've branched from your branch so you'll still get authorship credit

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