[rllib] Hyperparameter Optimisation Example

[pseudo-rnd-thoughts]

Description

This PR adds a HPO example to RLlib using HyperOpt and APPO + Cartpole.

Do we want this tested in nightly or premerge?

[gemini-code-assist]

Code Review

This pull request introduces a new example script for hyperparameter optimization using HyperOpt with APPO on the CartPole-v1 environment. The script is well-structured and provides a clear demonstration of using Ray Tune for HPO in RLlib. I've included a few minor suggestions to correct typos in the documentation and improve a hyperparameter range for better clarity and convention.

[kamil-kaczmarek]

Excellent example, showing RLlib and Ray Tune together!

I'd test this as a premerge, like this:

py_test(
    name = "learning_tests_ray_tune_cartpole_hyperopt_multi_gpu",
    size = "large",
    srcs = ["examples/ray_tune/cartpole_hyperopt.py"],
    args = [
        ...
    ],
    main = "examples/ray_tune/cartpole_hyperopt.py",
    tags = [
        "exclusive",
        "learning_tests",
        "multi_gpu",
        "team:rllib",
    ],
)

With multi_gpu tag we have 4 GPUs and 48 CPUs available, so we can run this HPO job on a single node cluster.

[kamil-kaczmarek]

Great example! Added few comments and suggested testing strategy.

[kamil-kaczmarek]

can run 4, this node is 4 GPU.

[pseudo-rnd-thoughts]

Updated to use 4 GPUs and 12 samples for 3 iterations

[kamil-kaczmarek]

Last details:

• need to bring back deps = [":conftest"], in the BUILD.bazel
• pytest has num-samples=12 samples, while python scripts has num-samples=16. I would parametrize the same in both places, to simplify. Either 12 or 16 looks good as long as we use the same number.
• I think you wanted to parametrize to run 3 iterations, but it's not in the code yet.
• In the previous PRs we agreed to list arg in the BUILD.bazel alphabetically. Can you fix here?

[pseudo-rnd-thoughts]

need to bring back deps = [":conftest"], in the BUILD.bazel

None of the examples use pytest therefore the conftest isn't actually used which is why I removed it. Why do you think we need it?

pytest has num-samples=12 samples, while python scripts has num-samples=16. I would parametrize the same in both places, to simplify. Either 12 or 16 looks good as long as we use the same number.

The python script doesn't have any performance testing, therefore, all we are testing is that script runs currently. To me, the difference in samples doesn't matter and just wastes premerge compute.

I think you wanted to parametrize to run 3 iterations, but it's not in the code yet.

What do you mean?

In the previous PRs we agreed to list arg in the BUILD.bazel alphabetically. Can you fix here?

Updated

@kamil-kaczmarek Checking premerge, this example is failing due to hyperopt not being installed.

[kamil-kaczmarek]

need to bring back deps = [":conftest"], in the BUILD.bazel

None of the examples use pytest therefore the conftest isn't actually used which is why I removed it. Why do you think we need it?

Ach right! Thought that you removed it from all py_tests.

pytest has num-samples=12 samples, while python scripts has num-samples=16. I would parametrize the same in both places, to simplify. Either 12 or 16 looks good as long as we use the same number.

The python script doesn't have any performance testing, therefore, all we are testing is that script runs currently. To me, the difference in samples doesn't matter and just wastes premerge compute.

My comment is about coherence/simplicity. How about 12 in both places?

I think you wanted to parametrize to run 3 iterations, but it's not in the code yet.

What do you mean?

I referred to your comment; you mentioned: "Updated to use 4 GPUs and 12 samples for 3 iterations".

In the previous PRs we agreed to list arg in the BUILD.bazel alphabetically. Can you fix here?

Updated

thx!

@kamil-kaczmarek Checking premerge, this example is failing due to hyperopt not being installed.

How about swapping HyperOpt with BasicVariantGenerator and still wrapping in ConcurrencyLimiter? I'd prefer not to add any extra dependencies to the testing image.

[kamil-kaczmarek]

LGTM!

[cursor]

Cursor Bugbot has reviewed your changes and found 1 potential issue.