[RLlib] [Release tests] Make rollout fragment length auto for release tests #30079
Conversation
Signed-off-by: Avnish <[email protected]>
| @@ -10,7 +10,7 @@ ddpg-hopperbulletenv-v0: | |||
| config: | |||
| actor_hiddens: [256, 256] | |||
| critic_hiddens: [256, 256] | |||
| n_step: 3 | |||
| n_step: 1 | |||
There was a problem hiding this comment.
Here, too, remove rollout_fragment_length (or set to "auto") further below entirely.
| @@ -26,7 +26,7 @@ cql-halfcheetahbulletenv-v0: | |||
| tau: 0.005 | |||
| target_entropy: auto | |||
| no_done_at_end: false | |||
| n_step: 3 | |||
| n_step: 1 | |||
There was a problem hiding this comment.
Wait, let's do this the other way around and keep the n_step setting.
So the new config logic is: rollout_fragment_length is "auto" b/c it's a lot of times dependent on other settings, like train_batch_size, num_envs_per_worker, and num_rollout_workers (PPO) or just needs to be at least n_step to make sense.
The RolloutWorkers now infer this setting from these other settings, if "auto", otherwise, take the user provided values, but this could lead to errors, like in this case.
So let's just remove the rollout_fragment_length setting entirely here (or set to its default: "auto").
| @@ -19,7 +19,7 @@ sac-halfcheetahbulletenv-v0: | |||
| tau: 0.005 | |||
| target_entropy: auto | |||
| no_done_at_end: false | |||
| n_step: 3 | |||
| n_step: 1 | |||
There was a problem hiding this comment.
Here, too, remove rollout_fragment_length (or set to "auto") further below entirely.
|
okie sg :) |
|
So I'm going to try this, but I suspect that we might need to increase the timestep limits on these tests in response, in order to meet the minimum reward threshold. |
Signed-off-by: Avnish <[email protected]>
…te_release_test_n_step_frag_length
Signed-off-by: Avnish <[email protected]>
|
lets merge pending the release tests all getting kicked off successfully: |
Signed-off-by: Avnish <[email protected]>
Signed-off-by: Avnish <[email protected]>
avnishn
changed the title
… tests (ray-project#30079) Signed-off-by: Avnish <[email protected]> Signed-off-by: Weichen Xu <[email protected]>
SAC, DDPG, and CQL release tests were broken since we started enforcing that the
rollout_fragment_lengthmust be greater than or equal ton_stepwhen computing n step returns.This error was triggered I think by #29854 and eventually propagated to the release tests. Its likely that validation wasn't being run properly before #29854 .
Signed-off-by: Avnish [email protected]
Why are these changes needed?
Related issue number
Checks
git commit -s) in this PR.scripts/format.shto lint the changes in this PR.