Use lr setter callback instead of `attr_name` in `LearningRateFinder` and `Tuner` about pytorch-lightning HOT 4 OPEN

As a callback you proposed this function that updates the lr argument in the partial wrapper:

def partial_setattr(fn: functools.partial, key: str, value: float) -> None:
    *_, (f, args, kwargs, n) = fn.__reduce__()
    kwargs[key] = value
    fn.__setstate__((f, args, kwargs, n))

My question is why couldn't you define your configure_optimizers() this way (pseudo code):

class LitModel:
    def __init__(self, optimizer_cls):
        self.optimizer_cls = optimizer_cls
        self.learning_rate = None  # or a default value

    def configure_optimizers(self):
        if self.learning_rate is not None:
            partial_setattr(self.optimizer_cls, "lr", self.learning_rate)
        return self.optimizer_cls(self.parameters())

from pytorch-lightning.

@arthurdjn Thanks for the feature request.
Currently I don't understand the use case 100%. My question is, since the contract between Trainer and LM is that the configure_optimizers() hook returns the optimizer, what would that implementation look like in your case? This is where you'd normally configure the learning rate. In other words, why couldn't that special callable you have be applied there?

The attribute the tuner saves the new learning rate to is only a temporary holder in that sense. Ultimately, it needs to be set in configure_optimizers().

from pytorch-lightning.

The use case is that I don't want to pass the learning rate as an attribute to the lightning module, instead I want to pass the partially instantiated optimizer. With this approach, it is not possible to use the Tuner or LearningRateFinder.

# Using the custom LitModel with partial optimizer instead of attr defined learning rate
optimizer = functools.partial(Adam, lr=0.001)
model = LitModel(optimizer)

trainer = Trainer()
tuner = Tuner(trainer)


# This does not work
lr_finder = tuner.lr_finder(model, attr_name=???) # Not possible to access the learning rate

Maybe I am missing something, but I think this use case is a current limitation of the API thus this feature proposal.

from pytorch-lightning.

Well, I agree that your solution is pretty simple and clean. I didn't want to have an extra learning_rate attribute in the model to avoid having the same value in different places. I thought that providing the setter function directly to the Tuner might be a clearer alternative. However, I think it's a good alternative.

What do you think?

from pytorch-lightning.