[tune] Fix durable(str) name for class trainables, preventing trial recovery

[krfricke]

Why are these changes needed?

When using tune.durable(str), a durable trainable is created from a registered trainable. The registry is a cluster-global KV=store, thus other nodes have access to it, even if they currently don't execute any code.

This trainable inherits the name of the previous trainable. However, tune then overwrites the string trainable in the cluster-gobal KV-store (Experiment.register_if_needed()).

It seems that this change is not correctly propagated to all nodes.

In effect, consider the following setup:

• A cluster with 4 nodes
• A run with 2 parallel trials using tune.durable("APPO") or similar
• One trial crashes after some time
• Tune tries to automatically recover the trial from the checkpoint
• Trial recovery happens on a different node than the one where the trial crashed.

For some reason, the new node creates a "APPO" trainable (a regular Trainable) and not the overwritten DurableTrainable. Thus, trial synchronization is not invoked and trial recovery failed because the checkpoint is not found.

Exactly why this happens is puzzling to me, as we don't schedule trainables by string reference, but by type reference in Ray Tune.

However, just changing the overwritten name to DurableAPPO fixes the issue reliably.

This is a follow-up to #19184, which introduced this change to function trainables, but not for class trainables.

cc @richardliaw @gjoliver

Edit: Maybe this has nothing to do with the KV-store. I'm not sure. I'll investigate further...

Related issue number

Checks

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

[gjoliver]

Fix looks totally reasonable.
Maybe add a TODO here about the fact that we shouldn't need this override?

[krfricke]

I added #19267 with a description of the source root of this problem to track and resolve it