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] Metrics do-over 03: Switch over Learner to new MetricsLogger API. #44995

Merged
merged 128 commits into from
May 1, 2024
Merged

[RLlib] Metrics do-over 03: Switch over Learner to new MetricsLogger API. #44995

merged 128 commits into from
May 1, 2024

Conversation

sven1977
Copy link
Contributor

Metrics do-over 03: Switch over Learner to new MetricsLogger API.

  • Retire Learner's current register_metric(s) APIs and replace with the new MetricsLogger API, which is also already used by EnvRunners AND Algorithm (if the new stack is activated).
  • TODO: We have to find a good way of transitioning those users that are currently on the hybrid stack and are therefore NOT using MetricsLogger on Algo yet (how to we reduce over the LearnerGroup returned reduced Stats-dicts). Possible solution: Have LearnerGroup act as an interceptor in case enable_rl_module_and_learner is True, but enable_env_runner_and_connector_v2 is False (hybrid stack). In this case, LearnerGroup will reduce the results on behalf of Algorithm and return a simple (non-Stats) dict, which hybrid-stack Algos know how to handle.

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 and others added 30 commits March 13, 2024 12:50
@sven1977
wip
6f1b505 
Signed-off-by: sven1977 <svenmika1977@gmail.com>
@sven1977
wip
b6e2714 
Signed-off-by: sven1977 <svenmika1977@gmail.com>
@sven1977
Merge branch 'master' of https://github.com/ray-project/ray into comp…
4dfb2ce 
…lete_metrics_and_stats_do_over
@sven1977
wip
e6402c6 
Signed-off-by: sven1977 <svenmika1977@gmail.com>
@sven1977
Merge branch 'master' of https://github.com/ray-project/ray into comp…
33487cc 
…lete_metrics_and_stats_do_over
@sven1977
doctest fix
a02abbd 
Signed-off-by: sven1977 <svenmika1977@gmail.com>
@sven1977
wip
d9f3e6e 
Signed-off-by: sven1977 <svenmika1977@gmail.com>
@sven1977
wip
e909a73 
Signed-off-by: sven1977 <svenmika1977@gmail.com>
@sven1977
wip
bdaa04c 
Signed-off-by: sven1977 <svenmika1977@gmail.com>
@sven1977
wip
52d9e12 
Signed-off-by: sven1977 <svenmika1977@gmail.com>
@sven1977
wip
f77ffdb 
Signed-off-by: sven1977 <svenmika1977@gmail.com>
@sven1977
Merge branch 'master' of https://github.com/ray-project/ray into clea…
81c4c79 
…nup_examples_folder_03

# Conflicts:
#	rllib/examples/multi_agent/multi_agent_pendulum.py

and wip

Signed-off-by: sven1977 <svenmika1977@gmail.com>
@sven1977
wip
1672675 
Signed-off-by: sven1977 <svenmika1977@gmail.com>
@sven1977
wip
adf9e8c 
Signed-off-by: sven1977 <svenmika1977@gmail.com>
@sven1977
wip
c9e5c2f 
Signed-off-by: sven1977 <svenmika1977@gmail.com>
@sven1977
wip
683bc4b 
Signed-off-by: sven1977 <svenmika1977@gmail.com>
@sven1977
wip
5bd220f 
Signed-off-by: sven1977 <svenmika1977@gmail.com>
@sven1977
LINT
d931945 
Signed-off-by: sven1977 <svenmika1977@gmail.com>
@sven1977
wip
e9888de 
Signed-off-by: sven1977 <svenmika1977@gmail.com>
@sven1977
fixes
0e97d8f 
Signed-off-by: sven1977 <svenmika1977@gmail.com>
@sven1977
fixes
7584cce 
Signed-off-by: sven1977 <svenmika1977@gmail.com>
@sven1977
Merge branch 'master' of https://github.com/ray-project/ray into comp…
5ba69af 
…lete_metrics_and_stats_do_over
@sven1977
wip
36bfa57 
Signed-off-by: sven1977 <svenmika1977@gmail.com>
@sven1977
fixes
5565d4f 
Signed-off-by: sven1977 <svenmika1977@gmail.com>
@sven1977
fixes
27c793d 
Signed-off-by: sven1977 <svenmika1977@gmail.com>
@sven1977
Merge branch 'master' of https://github.com/ray-project/ray into clea…
d75b31a 
…nup_examples_folder_03
@sven1977
fixes
bf9cef0 
Signed-off-by: sven1977 <svenmika1977@gmail.com>
@sven1977 @angelinalg
Apply suggestions from code review
872b49b 
Co-authored-by: angelinalg <122562471+angelinalg@users.noreply.github.com>
Signed-off-by: Sven Mika <sven@anyscale.io>
@sven1977
fixes
743dabd 
Signed-off-by: sven1977 <svenmika1977@gmail.com>
@sven1977
Merge remote-tracking branch 'origin/cleanup_examples_folder_03' into…
d50a39c 
… cleanup_examples_folder_03
@sven1977
fixes
e96efdc 
Signed-off-by: sven1977 <svenmika1977@gmail.com>
@sven1977
doctest fix
6c5627d 
Signed-off-by: sven1977 <svenmika1977@gmail.com>
@sven1977 sven1977 requested a review from a team as a code owner April 28, 2024 16:54
@sven1977
wip
dea6a34 
Signed-off-by: sven1977 <svenmika1977@gmail.com>
aslonnie
aslonnie approved these changes Apr 28, 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.

approval for docs.

@sven1977
wip
f7ae9ab 
Signed-off-by: sven1977 <svenmika1977@gmail.com>
@sven1977
DQN and SAC learn again
e3ff44f 
- after additional_update fixes/cleanups

Signed-off-by: sven1977 <svenmika1977@gmail.com>
@sven1977
doctest fix
1c8cbea 
Signed-off-by: sven1977 <svenmika1977@gmail.com>
@sven1977
fix
e21b5f2 
Signed-off-by: sven1977 <svenmika1977@gmail.com>
@sven1977
Merge branch 'master' of https://github.com/ray-project/ray into metr…
ea38d03 
…ics_do_over_03_learner_on_new_metrics_logger

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

# Conflicts:
#	rllib/algorithms/dqn/dqn.py
#	rllib/algorithms/dreamerv3/tf/dreamerv3_tf_learner.py
#	rllib/algorithms/sac/torch/sac_torch_learner.py
#	rllib/core/learner/learner.py
#	rllib/core/learner/tests/test_learner_group.py
@sven1977
Merge branch 'master' of https://github.com/ray-project/ray into metr…
b4e5a79 
…ics_do_over_03_learner_on_new_metrics_logger
@sven1977
wip
6089c10 
Signed-off-by: sven1977 <svenmika1977@gmail.com>
@sven1977
fix
de50b67 
Signed-off-by: sven1977 <svenmika1977@gmail.com>
@sven1977
fixes most test cases
6ba560d 
- added tensor-mode to MetricsLogger, during which tensors can be logged.
- tensor-mode requires to be exited (upon which a tensor dict is returned with the numpy/eager values in the tensors).
- these eager/numpy values must then be assigned again into the MetricsLogger, so we can properly reduce.

Signed-off-by: sven1977 <svenmika1977@gmail.com>
@sven1977
merge
245f9bd 
Signed-off-by: sven1977 <svenmika1977@gmail.com>
@sven1977
LINT
ac7470c 
Signed-off-by: sven1977 <svenmika1977@gmail.com>
@sven1977
Merge branch 'master' of https://github.com/ray-project/ray into metr…
902818d 
…ics_do_over_03_learner_on_new_metrics_logger

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

# Conflicts:
#	rllib/algorithms/ppo/ppo_learner.py
#	rllib/algorithms/ppo/tf/ppo_tf_learner.py
#	rllib/algorithms/ppo/torch/ppo_torch_learner.py
@sven1977
wip
0f19aa1 
Signed-off-by: sven1977 <svenmika1977@gmail.com>
@sven1977
fixes
3a677c5 
Signed-off-by: sven1977 <svenmika1977@gmail.com>
@sven1977
Merge branch 'master' of https://github.com/ray-project/ray into metr…
2fae0ca 
…ics_do_over_03_learner_on_new_metrics_logger
@sven1977
Merge branch 'master' of https://github.com/ray-project/ray into metr…
720340c 
…ics_do_over_03_learner_on_new_metrics_logger
@sven1977
fixes
9242c24 
Signed-off-by: sven1977 <svenmika1977@gmail.com>
@sven1977
fixes
4fa2edb 
Signed-off-by: sven1977 <svenmika1977@gmail.com>
@sven1977
fixed multi-gpu test cases.
3110a56 
Signed-off-by: sven1977 <svenmika1977@gmail.com>
@sven1977
fixes
9ae2204 
Signed-off-by: sven1977 <svenmika1977@gmail.com>
@sven1977 sven1977 merged commit 8f7dd2e into ray-project:master May 1, 2024
4 of 5 checks passed
@sven1977 sven1977 deleted the metrics_do_over_03_learner_on_new_metrics_logger branch May 1, 2024 19:13
harborn pushed a commit to harborn/ray that referenced this pull request May 8, 2024
@sven1977 @harborn
[RLlib] Metrics do-over 03: Switch over Learner to new MetricsLogge…
2d58704 
…r API. (ray-project#44995)
ryanaoleary pushed a commit to ryanaoleary/ray that referenced this pull request Jun 6, 2024
@sven1977 @ryanaoleary
[RLlib] Metrics do-over 03: Switch over Learner to new MetricsLogge…
50ce011 
…r API. (ray-project#44995)

Signed-off-by: Ryan O'Leary <ryanaoleary@google.com>
ryanaoleary pushed a commit to ryanaoleary/ray that referenced this pull request Jun 7, 2024
@sven1977 @ryanaoleary
[RLlib] Metrics do-over 03: Switch over Learner to new MetricsLogge…
745230c 
…r API. (ray-project#44995)
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 approved these changes

@avnishn avnishn Awaiting requested review from avnishn

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

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

@kouroshHakha kouroshHakha Awaiting requested review from kouroshHakha

Assignees

@simonsays1980 simonsays1980

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@sven1977 @simonsays1980 @aslonnie