[tune] Fix BOHB example for new storage

[krfricke]

Why are these changes needed?

The new storage path does not create "empty" checkpoints per default anymore. Previously, when no checkpoint is saved, PAUSEing a trial would create a dummy checkpoint that only contains trial metadata (such as the iteration number). This is not the case anymore.

Examples now have to implement checkpointing to properly restore previous state. This was also true previously - but some of our simple examples (e.g. the one in this PR) didn't implement it and still "worked".

I think it's fine to keep the functionality as is and require our examples to show checkpointing implementations. This will ensure that users don't shoot their feet trying to use e.g. BOHB.

Separately, BOHB was malfunctioning as trials were repeatedly PAUSED and restarted as they've never been removed from bracket.trials_to_unpause. @justinvyu mentioned this in the review where it was introduced and I believed at the time it wasn't necessary - turns out it is, as we can end up in a situation where a bracket is never finished because trials are constantly running. This was not caught by any tests. We should add one in a follow-up - for now we can proceed with this PR to pick onto Ray 2.7.

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

[justinvyu]

Thanks! 1 suggestion

[justinvyu]

I see -- is this the only place that needs to be updated?

[krfricke]

I'm actually surprised this is not happening in more cases (and why it didn't break before) - class trainables default to checkpoint_at_end=True and this can raise a NotImplementedError.

I think there are two options to solve this. 1) Default to False even for class trainables, or 2) Only set True if checkpointing is implemented (method is overwritten).

[justinvyu]

Looked into this and turns out the default checkpoint_at_end is actually None.

Only in the Tuner do we set it to True for class trainables. In tune.run it just stays as None and no checkpoint happens at the end.

from ray import tune


class Test(tune.Trainable):
    def step(self):
        return {"done": True}

tune.run(Test)  # works
tune.Tuner(Test).fit()  # errors

[justinvyu]

LGTM, let's fix that checkpoint_at_end issue 🤯