Skip to content

Conversation

@sven1977
Copy link
Contributor

@sven1977 sven1977 commented May 16, 2024

Cleanup examples folder #13. Fix main examples docs page for RLlib.

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 added 13 commits May 16, 2024 04:33
Signed-off-by: sven1977 <[email protected]>
Signed-off-by: sven1977 <[email protected]>
Signed-off-by: sven1977 <[email protected]>
Signed-off-by: sven1977 <[email protected]>
Signed-off-by: sven1977 <[email protected]>
Signed-off-by: sven1977 <[email protected]>
Signed-off-by: sven1977 <[email protected]>
…p_examples_folder_11_fractional_gpus

# Conflicts:
#	rllib/utils/test_utils.py
Signed-off-by: sven1977 <[email protected]>
Signed-off-by: sven1977 <[email protected]>
@sven1977 sven1977 added rllib RLlib related issues rllib-docs-or-examples Issues related to RLlib documentation or rllib/examples rllib-newstack labels May 16, 2024
Signed-off-by: sven1977 <[email protected]>
Copy link
Contributor

@angelinalg angelinalg left a comment

Choose a reason for hiding this comment

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

Just some style nits and a few typos.

`num_gpus_per_worker` to 0 (they may be set to 1 by default for your
particular RL algorithm)."""
machine does not have any GPUs, you should set the config keys
`num_gpus_per_learner` and `num_gpus_per_env_runner` to 0 (they may be set to
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
`num_gpus_per_learner` and `num_gpus_per_env_runner` to 0 (they may be set to
`num_gpus_per_learner` and `num_gpus_per_env_runner` to 0. They may be set to

particular RL algorithm)."""
machine does not have any GPUs, you should set the config keys
`num_gpus_per_learner` and `num_gpus_per_env_runner` to 0 (they may be set to
1 by default for your particular RL algorithm)."""
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
1 by default for your particular RL algorithm)."""
1 by default for your particular RL algorithm."""

1 by default for your particular RL algorithm)."""

ERR_MSG_INVALID_ENV_DESCRIPTOR = """The env string you provided ('{}') is:
a) Not a supported/installed environment.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
a) Not a supported/installed environment.
a) Not a supported or installed environment.

trainable: The Trainable sub-class to run in the tune.Tuner. If None (default),
use the registered RLlib Algorithm class specified by args.algo.
tune_callbacks: A list of Tune callbacks to configure with the tune.Tuner.
In case `args.wandb_key` is provided, will append a WandB logger to this
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
In case `args.wandb_key` is provided, will append a WandB logger to this
In case `args.wandb_key` is provided, appends a WandB logger to this

keep_config: Set this to True, if you don't want this utility to change the
given `base_config` in any way and leave it as-is. This is helpful
for example script that want to demonstrate how to set those settings
that are usually taken care of automatically in this function (e.g.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
that are usually taken care of automatically in this function (e.g.
that are usually taken care of automatically in this function (e.g.,

sven1977 added 3 commits June 3, 2024 21:23
…nup_examples_folder_13_folder_readme

Signed-off-by: sven1977 <[email protected]>

# Conflicts:
#	doc/source/rllib/rllib-advanced-api.rst
#	doc/source/rllib/rllib-learner.rst
#	rllib/BUILD
#	rllib/algorithms/algorithm.py
#	rllib/algorithms/algorithm_config.py
#	rllib/algorithms/dreamerv3/tests/test_dreamerv3.py
#	rllib/core/learner/learner.py
#	rllib/core/learner/scaling_config.py
#	rllib/examples/checkpoints/restore_1_of_n_agents_from_checkpoint.py
#	rllib/examples/gpus/fractional_gpus_per_learner.py
#	rllib/tuned_examples/dreamerv3/atari_100k.py
#	rllib/tuned_examples/dreamerv3/atari_200M.py
#	rllib/tuned_examples/dreamerv3/dm_control_suite_vision.py
#	rllib/utils/test_utils.py
Signed-off-by: sven1977 <[email protected]>
sven1977 and others added 12 commits June 10, 2024 12:21
Signed-off-by: sven1977 <[email protected]>
Signed-off-by: sven1977 <[email protected]>
Co-authored-by: angelinalg <[email protected]>
Signed-off-by: Sven Mika <[email protected]>
Co-authored-by: angelinalg <[email protected]>
Signed-off-by: Sven Mika <[email protected]>
Co-authored-by: angelinalg <[email protected]>
Signed-off-by: Sven Mika <[email protected]>
Signed-off-by: sven1977 <[email protected]>
…r_readme' into cleanup_examples_folder_13_folder_readme
Signed-off-by: sven1977 <[email protected]>
Signed-off-by: sven1977 <[email protected]>
Signed-off-by: sven1977 <[email protected]>
@sven1977 sven1977 changed the title [RLlib] Cleanup examples folder 13: Add READMEs to folder and all sub-folders. [RLlib] Cleanup examples folder #13. Fix main examples docs page for RLlib. Jun 11, 2024
sven1977 added 3 commits June 11, 2024 14:15
Signed-off-by: sven1977 <[email protected]>
…nup_examples_folder_13_folder_readme

Signed-off-by: sven1977 <[email protected]>

# Conflicts:
#	rllib/examples/inference/policy_inference_after_training.py
Signed-off-by: sven1977 <[email protected]>
@sven1977 sven1977 enabled auto-merge (squash) June 11, 2024 13:52
@github-actions github-actions bot added the go add ONLY when ready to merge, run all tests label Jun 11, 2024
Copy link
Contributor

@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.

RLlib uses `Ray actors <actors.html>`__ to scale training from a single core to many thousands of cores in a cluster.
You can `configure the parallelism <rllib-training.html#specifying-resources>`__ used for training by changing the ``num_env_runners`` parameter.
Check out our `scaling guide <rllib-training.html#scaling-guide>`__ for more details here.
See this `scaling guide <rllib-training.html#scaling-guide>`__ for more details here.
Copy link
Contributor

Choose a reason for hiding this comment

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

The scaling guide also needs to be overhauled.

an index into the available
CUDA devices. For example if `os.environ["CUDA_VISIBLE_DEVICES"] = "1"`
then a `local_gpu_idx` of 0 will use the GPU with ID=1 on the node.
and `local_gpu_idx=0`, RLlib uses the GPU with ID=1 on the node.
Copy link
Contributor

Choose a reason for hiding this comment

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

This feels counterintuitive. The GPU index 0 does not equal the environment variable 1 and we have two or more GPUs for a single learner. A user would expect a single GPU for a single learner when multiple GPUs are available on a node to be indicated with an ID or index. Do I misunderstand sth here?

`num_learners` x `train_batch_size_per_learner` and can
be accessed via the property `AlgorithmConfig.total_train_batch_size`.
`num_learners` x `train_batch_size_per_learner` and you can
access it with the property `AlgorithmConfig.total_train_batch_size`.
Copy link
Contributor

Choose a reason for hiding this comment

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

We should refer hereto in the scaling guide ~ if not done yet.

@github-actions github-actions bot disabled auto-merge June 12, 2024 10:03
Signed-off-by: sven1977 <[email protected]>
@sven1977 sven1977 enabled auto-merge (squash) June 12, 2024 11:21
@sven1977 sven1977 merged commit c84bf37 into ray-project:master Jun 12, 2024
@sven1977 sven1977 deleted the cleanup_examples_folder_13_folder_readme branch June 12, 2024 12:59
richardsliu pushed a commit to richardsliu/ray that referenced this pull request Jun 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

go add ONLY when ready to merge, run all tests rllib RLlib related issues rllib-docs-or-examples Issues related to RLlib documentation or rllib/examples

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants