Skip to content

[RLlib] PPO runs with EnvRunner w/o old Policy API (also solves KL issues with PPORLModules).#39732

Merged
sven1977 merged 57 commits intoray-project:masterfrom
simonsays1980:solve-kl-issues-with-ppo-rl-module
Oct 25, 2023
Merged

[RLlib] PPO runs with EnvRunner w/o old Policy API (also solves KL issues with PPORLModules).#39732
sven1977 merged 57 commits intoray-project:masterfrom
simonsays1980:solve-kl-issues-with-ppo-rl-module

Conversation

@simonsays1980
Copy link
Contributor

@simonsays1980 simonsays1980 commented Sep 18, 2023 •
edited
Loading

Why are these changes needed?

By time sampling should be performed by the env.EnvRunner class, individually for different algorithms. In the same breath the policy should become obsolete.

Following the example of DreamerV3 this draft PR should develop a way to implement these changes into PPO.

Related issue number

Closes #39174 #39813

Checks

  • I've signed off every commit(by using the -s flag, i.e., git commit -s) in this PR.
  • I've run scripts/format.sh to lint the changes in this PR.
  • I've included any doc changes needed for https://docs.ray.io/en/master/.
    • I've added any new APIs to the API Reference. For example, if I added a
      method in Tune, I've added it in doc/source/tune/api/ under the
      corresponding .rst file.
  • I've made sure the tests are passing. Note that there might be a few flaky tests, see the recent failures at https://flakey-tests.ray.io/
  • Testing Strategy
    • Unit tests
    • Release tests
    • This PR is not tested :(

@simonsays1980
Added PPOEnvRunner and added implementations to PPO.
2762b77 
Signed-off-by: Simon Zehnder <simon.zehnder@gmail.com>
@simonsays1980
Run linter.
041d4c2 
Signed-off-by: Simon Zehnder <simon.zehnder@gmail.com>
@simonsays1980
Run linter.
0d0f0dc 
Signed-off-by: Simon Zehnder <simon.zehnder@gmail.com>
@simonsays1980
Merge branch 'master' into solve-kl-issues-with-ppo-rl-module
1192243 
Signed-off-by: Simon Zehnder <simon.zehnder@gmail.com>
@simonsays1980
Added EnvRunner to PPO. Added foreach_module to MARL and modified set…
dd679fe 
…_weigths, get_weigths from EnvRunner such that EnvRunner can be used equivalently to RolloutWorker in Algorithm.

Signed-off-by: Simon Zehnder <simon.zehnder@gmail.com>
@simonsays1980
Had to run linter.
c0bb25d 
Signed-off-by: Simon Zehnder <simon.zehnder@gmail.com>
@simonsays1980
Moved 'foreach_module' to utils.rl_module.py'.
0a67532 
Signed-off-by: Simon Zehnder <simon.zehnder@gmail.com>
@simonsays1980
Merge branch 'master' into solve-kl-issues-with-ppo-rl-module
8fd93f9 
Signed-off-by: Simon Zehnder <simon.zehnder@gmail.com>
@simonsays1980
EnvRunner needed some modifications as well as PPO. Sampling works no…
d5875b1 
…w. _Episode needs some fixing for some algorithms as they need extra keys in the sample batch.

Signed-off-by: Simon Zehnder <simon.zehnder@gmail.com>
@simonsays1980
Running linter.
77b0036 
Signed-off-by: Simon Zehnder <simon.zehnder@gmail.com>
simonsays1980
simonsays1980 commented Sep 18, 2023
View reviewed changes
@sven1977 sven1977 changed the title Solve kl issues with ppo rl module [RLlib] Solve KL issues with PPORLModules. Sep 20, 2023
sven1977
sven1977 reviewed Sep 20, 2023
View reviewed changes
sven1977
sven1977 reviewed Sep 20, 2023
View reviewed changes
sven1977
sven1977 reviewed Sep 20, 2023
View reviewed changes
sven1977
sven1977 reviewed Sep 20, 2023
View reviewed changes
sven1977
sven1977 reviewed Sep 20, 2023
View reviewed changes
sven1977
sven1977 reviewed Sep 20, 2023
View reviewed changes
sven1977
sven1977 reviewed Sep 20, 2023
View reviewed changes
sven1977
sven1977 reviewed Sep 20, 2023
View reviewed changes
sven1977
sven1977 reviewed Sep 20, 2023
View reviewed changes
sven1977
sven1977 reviewed Sep 20, 2023
View reviewed changes
@simonsays1980
Merge branch 'master' into solve-kl-issues-with-ppo-rl-module
4066431 
Signed-off-by: Simon Zehnder <simon.zehnder@gmail.com>
@simonsays1980
Added 'info' to '_Episode'. Wrapped vector env into VecotListInfo to …
8de01f9 
…get infos as lists.

Signed-off-by: Simon Zehnder <simon.zehnder@gmail.com>
@simonsays1980
Added extra_model_outputs to the _Episode. Started to create postproc…
5a9c823 
…essing for episodes instead of SampleBatches.

Signed-off-by: Simon Zehnder <simon.zehnder@gmail.com>
@sven1977 sven1977 mentioned this pull request Sep 21, 2023
Closed
@simonsays1980
Implemented bootstrapping for GAE on episodes. Run linter.
346cbcd 
Signed-off-by: Simon Zehnder <simon.zehnder@gmail.com>
@simonsays1980
Added advantage computation to EnvRunner and added tqdm to EnvRunner.…
442f9cd 
… Training works now.

Signed-off-by: Simon Zehnder <simon.zehnder@gmail.com>
@simonsays1980
Added advantage computation to EnvRunner and added tqdm to EnvRunner.…
0924207 
… Training works now.

Signed-off-by: Simon Zehnder <simon.zehnder@gmail.com>
@simonsays1980
Implemented changes from @sven1977 review. Moved 'foreach_module()' t…
4e8fb06 
…o 'MultiAgentRLModule'. Created cases for 'num_remote_workers() <=0', kept logic for 'ModelV2' logic. Removed global vars for 'Learner API' in 'PPO' training step. Test 'ppo_with_rl_module()' runs. Had to remove for this 'check_compute_single_action_from_input_dict()' as 'EnvRunner' has no policy.

Signed-off-by: Simon Zehnder <simon.zehnder@gmail.com>
simonsays1980 and others added 9 commits October 6, 2023 17:12
@simonsays1980
Implemented review from @sven1977. Furthermore moved postprocessing t…
e3bf8f0 
…o PPO from SingleAgentEnvRunner. Implemented logic for torch and tf2. Test runs. Tuned example not yet.

Signed-off-by: Simon Zehnder <simon.zehnder@gmail.com>
@simonsays1980
Solved conflicts and removed unused imports.
2120e75 
Signed-off-by: Simon Zehnder <simon.zehnder@gmail.com>
@simonsays1980
Ran linter.
db0b93d 
Signed-off-by: Simon Zehnder <simon.zehnder@gmail.com>
@simonsays1980
Fixed minor bugs.
6c016a2 
Signed-off-by: Simon Zehnder <simon.zehnder@gmail.com>
@simonsays1980
Added async evaluation.
d8de7f6 
Signed-off-by: Simon Zehnder <simon.zehnder@gmail.com>
@simonsays1980
Removed initialized episodes from sampling output to fix 'vf_preds' k…
149c5ea 
…ey error in postprocessing. Furthermore, modified weight synching to synch not too often.

Signed-off-by: Simon Zehnder <simon.zehnder@gmail.com>
@simonsays1980
Changed import to original one in postprocessing.
6eef936 
Signed-off-by: Simon Zehnder <simon.zehnder@gmail.com>
@sven1977
wip
d6f1b58 
Signed-off-by: sven1977 <svenmika1977@gmail.com>
@sven1977
Merge branch 'solve-kl-issues-with-ppo-rl-module' of https://github.c…
5deaa74 
…om/simonsays1980/ray into solve-kl-issues-with-ppo-rl-module
@ArturNiederfahrenhorst ArturNiederfahrenhorst mentioned this pull request Oct 17, 2023
Open
@sven1977 sven1977 mentioned this pull request Oct 18, 2023
Open
simonsays1980 and others added 6 commits October 23, 2023 10:07
@simonsays1980
Debugging PPO with EnvRunner.
8a3621c 
Signed-off-by: Simon Zehnder <simon.zehnder@gmail.com>
@simonsays1980
Debugging PPO with EnvRunner. Added test environments.
82f9152 
Signed-off-by: Simon Zehnder <simon.zehnder@gmail.com>
@sven1977
wip
4bdf1a8 
Signed-off-by: sven1977 <svenmika1977@gmail.com>
@sven1977
Merge branch 'master' of https://github.com/ray-project/ray into solv…
e12b356 
…e-kl-issues-with-ppo-rl-module
@sven1977
wip
f379269 
Signed-off-by: sven1977 <svenmika1977@gmail.com>
@sven1977
wip
dffc9fb 
Signed-off-by: sven1977 <svenmika1977@gmail.com>
sven1977
sven1977 approved these changes Oct 24, 2023
View reviewed changes
Copy link
Contributor

@sven1977 sven1977 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome work @simonsays1980!
Thanks for thie great PR. Should be all easily rolling downhill from here on :)

@sven1977
LINT
c124d98 
Signed-off-by: sven1977 <svenmika1977@gmail.com>
@sven1977
wip
8d8119d 
Signed-off-by: sven1977 <svenmika1977@gmail.com>
@sven1977
wip
fac1815 
Signed-off-by: sven1977 <svenmika1977@gmail.com>
@simonsays1980
Copy link
Contributor Author

simonsays1980 commented Oct 24, 2023

Happy for the contribution. I am excited about the new sampling API and how it will improve learning performance and user experience. Thanks for the great input @sven1977, @ArturNiederfahrenhorst and @kouroshHakha

@sven1977
wip
e011776 
Signed-off-by: sven1977 <svenmika1977@gmail.com>
@sven1977
wip
eba5667 
Signed-off-by: sven1977 <svenmika1977@gmail.com>
@sven1977
wip
069979c 
Signed-off-by: sven1977 <svenmika1977@gmail.com>
@sven1977
wip
fc3d387 
Signed-off-by: sven1977 <svenmika1977@gmail.com>
@sven1977
wip
aa469de 
Signed-off-by: sven1977 <svenmika1977@gmail.com>
@sven1977
Merge branch 'master' of https://github.com/ray-project/ray into solv…
6c49aca 
…e-kl-issues-with-ppo-rl-module
@sven1977
wip
e5b01a3 
Signed-off-by: sven1977 <svenmika1977@gmail.com>
@sven1977 sven1977 merged commit 41714dd into ray-project:master Oct 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@sven1977 sven1977 sven1977 approved these changes

@gjoliver gjoliver Awaiting requested review from gjoliver

@avnishn avnishn Awaiting requested review from avnishn

@ArturNiederfahrenhorst ArturNiederfahrenhorst Awaiting requested review from ArturNiederfahrenhorst

@smorad smorad Awaiting requested review from smorad

@maxpumperla maxpumperla Awaiting requested review from maxpumperla

@kouroshHakha kouroshHakha Awaiting requested review from kouroshHakha

@krfricke krfricke Awaiting requested review from krfricke

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[RLlib] Issues with RLModules/Learner + evaluation workers and not using KL loss

2 participants

@simonsays1980 @sven1977