[Tune] Fix validation error message and docstring of num_to_keep in CheckpointConfig

[justinvyu]

Signed-off-by: Justin Yu justinvyu@berkeley.edu

Why are these changes needed?

Background

When reading the CheckpointConfig docstring, it seems like num_to_keep=0 is a valid configuration, where no persistent checkpoints will be kept.

        num_to_keep: The number of checkpoints to keep
            on disk for this run. If a checkpoint is persisted to disk after
            there are already this many checkpoints, then an existing
            checkpoint will be deleted. If this is ``None`` then checkpoints
            will not be deleted. If this is ``0`` then no checkpoints will be
            persisted to disk.

However, when setting num_to_keep=0 in the CheckpointConfig through either an AIR Trainer or the Tuner, the following error will be thrown:

RuntimeError: If checkpointing is enabled, Ray Tune requires `keep_checkpoints_num` to be None or a number greater than 0

Historically, the old Ray Train allowed num_to_keep=0. However, Tune requires at least one persistent checkpoint for fault tolerance, and the Ray Train in AIR now runs as a Tune experiment, so num_to_keep >= 1 is needed in both cases.

Summary

This PR makes the following fixes:

1. The CheckpointConfig docstring should accurately reflect the requirement that num_to_keep >= 1.
2. keep_checkpoints_num in the error should be num_to_keep instead, since the Tune checkpoint manager takes in a CheckpointConfig now.

Related issue number

Clarifies confusion from the documentation as referenced here: #30506

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

[Yard1]

Looks good, though I think it would make more sense to change the actual validation logic here and have a subclass without it for testing. This can get confusing as the error message is now diverged from the logic.

[Yard1]

LGTM

[krfricke]

Thanks!