[Tune] [Train] Only convert a BaseTrainer to Trainable once in the Tuner

[justinvyu]

Signed-off-by: Justin Yu [email protected]

Why are these changes needed?

When using Tune with Train (Tuner(trainer)), the trainer is currently being converted to a Tune Trainable multiple times. This is possibly expensive, since BaseTrainer.as_trainable will put all of its config (which could contain a large checkpointed model in resume_from_checkpoint) into the object store from a call to tune.with_parameters. This PR makes it so that the conversion only happens once.

Related issue number

Closes #30321

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]

Thanks, this is great. To be extra safe, I'd propose to consider using properties to link _trainable and _converted_trainable together:

    @property
    def _trainable(self):
        return self.__trainable

    @property
    def _converted_trainable(self):
        return self.__converted_trainable

    @_trainable.setter
    def _trainable(self, value):
        self.__trainable = value
        self.__converted_trainable = self._convert_trainable(self.__trainable)

(and change the keys in constants to account for extra underscores)

My worry (even if that's a very unlikely situation) is that we can end up with a situation where _trainable has been changed but the cached _converted_trainable is still being used.

Let me know what you think!

[justinvyu]

This makes sense, I'll go ahead and make the change!

[justinvyu]

One thing that I had to change was:

• Python automatically adds the class name to double underscore attributes when accessing the __dict__. So, the __trainable attribute would have the key "_TunerInternal__trainable" in __getstate__. Then, the key to pop the attribute would be "_TunerInternal__trainable", which is a bit confusing.
• I changed the properties to self.trainable and self.converted_trainable instead, with _trainable and _converted_trainable as the private variables.

[Yard1]

LGTM, thanks!