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.

Sign up for GitHub

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

Jump to bottom

[RLlib] Activate DreamerV3 weekly release test (on Pong-v5 with the 100k setup). #45654

Merged
merged 12 commits into from
Jun 4, 2024
Merged

[RLlib] Activate DreamerV3 weekly release test (on Pong-v5 with the 100k setup). #45654

merged 12 commits into from
Jun 4, 2024

Conversation

sven1977
Copy link
Contributor

@sven1977 sven1977 commented May 31, 2024 •
edited
Loading

Activate DreamerV3 weekly release test (on Pong-v5 with the 100k setup).

Why are these changes needed?

Related issue number

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 :(

@sven1977
wip
d79cfb1 
Signed-off-by: sven1977 <svenmika1977@gmail.com>
@sven1977
wip
63a969e 
Signed-off-by: sven1977 <svenmika1977@gmail.com>
simonsays1980
simonsays1980 approved these changes May 31, 2024
View reviewed changes
Copy link
Collaborator

@simonsays1980 simonsays1980 left a comment

Choose a reason for hiding this comment

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

LGTM: Very excited about the tests.

rllib/tuned_examples/dreamerv3/atari_100k.py Outdated
)
.env_runners(
num_env_runners=(args.num_env_runners or 0),
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why is it that we use only a single env runner to collect new samples?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The training ratio for this setup (Atari 100k) is 1024, which is huge anyways.
You sample 1 step, you update all models on one 1024 sized batch (B=16 x T=64) from the buffer.

So parallelizing the env collection does not make sense. You don't win any performance out of this. DreamerV3 is all about parallelizing on the learner side.

Copy link
Collaborator

@simonsays1980 simonsays1980 May 31, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Interesting, I would have thought that you learn more about the environment if you roll out the new policy more broadly, which in turn would have improved the dynamics model faster to dream better.

Thanks for the clarification @sven1977

can-anyscale
can-anyscale approved these changes May 31, 2024
View reviewed changes
release/release_tests.yaml
runtime_env:
- RLLIB_TEST_NO_JAX_IMPORT=1
- LD_LIBRARY_PATH=$LD_LIBRARY_PATH:/home/ray/.mujoco/mujoco210/bin
cluster_compute: 1gpu_4cpus.yaml
Copy link
Collaborator

Choose a reason for hiding this comment

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

this is using p2.xlarge which is about $1/hour; from cost perspective, i think it's absolutely fine to use this node for 12 hours per week

from the weekly release perspective, we are thinking about reduce many 24-hour tests to be 8-hour tests, but no concrete plan yet so should not be a blocker

@sven1977 sven1977 enabled auto-merge (squash) May 31, 2024 13:49
@sven1977 sven1977 disabled auto-merge May 31, 2024 13:49
@github-actions github-actions bot added the go add ONLY when ready to merge, run all tests label May 31, 2024
@sven1977 sven1977 added the tests-ok The tagger certifies test failures are unrelated and assumes personal liability. label May 31, 2024
@sven1977
tf==2.11.1
e9332de 
Signed-off-by: sven1977 <svenmika1977@gmail.com>
@sven1977 sven1977 requested a review from a team as a code owner May 31, 2024 15:25
@sven1977
fix
6f9499e 
Signed-off-by: sven1977 <svenmika1977@gmail.com>
aslonnie
aslonnie reviewed May 31, 2024
View reviewed changes
Copy link
Collaborator

@aslonnie aslonnie left a comment

Choose a reason for hiding this comment

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

(leaving for @can-anyscale to review the release test)

@sven1977
wip
e54d905 
Signed-off-by: sven1977 <svenmika1977@gmail.com>
@sven1977
Merge branch 'master' of https://github.com/ray-project/ray into drea…
18275a2 
…mer_v3_add_release_test

Signed-off-by: sven1977 <svenmika1977@gmail.com>

# Conflicts:
#	rllib/tuned_examples/dreamerv3/atari_100k.py
@sven1977
wip
84c7ea3 
Signed-off-by: sven1977 <svenmika1977@gmail.com>
@sven1977
Merge branch 'master' of https://github.com/ray-project/ray into drea…
0748415 
…mer_v3_add_release_test
@sven1977
test removing byod file altogether
9e3987d 
Signed-off-by: sven1977 <svenmika1977@gmail.com>
@sven1977
fix
73f882f 
Signed-off-by: sven1977 <svenmika1977@gmail.com>
@sven1977
wip
8a8f43f 
Signed-off-by: sven1977 <svenmika1977@gmail.com>
@sven1977
Merge branch 'master' into dreamer_v3_add_release_test
d42ec0f
@sven1977 sven1977 enabled auto-merge (squash) June 4, 2024 05:43
@sven1977 sven1977 merged commit 4adb78b into ray-project:master Jun 4, 2024
6 checks passed
ryanaoleary pushed a commit to ryanaoleary/ray that referenced this pull request Jun 6, 2024
@sven1977 @ryanaoleary
[RLlib] Activate DreamerV3 weekly release test (on Pong-v5 with the 1…
b789067 
…00k setup). (ray-project#45654)

Signed-off-by: Ryan O'Leary <ryanaoleary@google.com>
ryanaoleary pushed a commit to ryanaoleary/ray that referenced this pull request Jun 6, 2024
@sven1977 @ryanaoleary
[RLlib] Activate DreamerV3 weekly release test (on Pong-v5 with the 1…
f4448b3 
…00k setup). (ray-project#45654)

Signed-off-by: Ryan O'Leary <ryanaoleary@google.com>
richardsliu pushed a commit to richardsliu/ray that referenced this pull request Jun 12, 2024
@sven1977 @richardsliu
[RLlib] Activate DreamerV3 weekly release test (on Pong-v5 with the 1…
f164741 
…00k setup). (ray-project#45654)

Signed-off-by: Richard Liu <ricliu@google.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@simonsays1980 simonsays1980 simonsays1980 approved these changes

@aslonnie aslonnie aslonnie left review comments

@can-anyscale can-anyscale can-anyscale approved these changes

@ArturNiederfahrenhorst ArturNiederfahrenhorst Awaiting requested review from ArturNiederfahrenhorst ArturNiederfahrenhorst is a code owner

Assignees

@simonsays1980 simonsays1980

@can-anyscale can-anyscale

Labels
go add ONLY when ready to merge, run all tests tests-ok The tagger certifies test failures are unrelated and assumes personal liability.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@sven1977 @simonsays1980 @aslonnie @can-anyscale