-
Notifications
You must be signed in to change notification settings - Fork 3.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Use lr setter callback instead of attr_name
in LearningRateFinder
and Tuner
#19949
Comments
@arthurdjn Thanks for the feature request. 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 |
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. |
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 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()) |
Well, I agree that your solution is pretty simple and clean. I didn't want to have an extra What do you think? |
Description & Motivation
I would like to change the the
Tuner
andLearningRateFinder
API so that it is possible to use more custom models.Description
Currently, the learning rate can only be accessed from the
LightningModule
model through an attribute name (either as attribute or within the hyper parametershparams
). This can be configured through theattr_name
parameter.However, I would like to replace the
attr_name
parameter with a callbacklr_setter
to allow advanced access, customization and freedom on where the learning rate is located inside the model.Motivation
While the current implementation will suit most use cases, it does not fit some advanced usage. Let's say I want to provide the learning rater through a partially instantiated optimizer. This works very well in a hydra / conf setup.
For example, I find the use of partially instantiated optimizer really helpful for tracking experiments, etc. I often use something like:
With this implementation it is not possible to use the
LearningRateFinder
callback orTuner
because the learning rate is not accessible through an attribute or the hyper parameters.Pitch
I would like to change the parameter
attr_name
tolr_setter
(or similar), which could be a function that sets the learning rate given the model. With that functionality it could be possible to use theTuner
andLearningRateFinder
in more advanced cases, while being compatible with attr-defined learning rate:The type of the
lr_setter
could be aCallable[[pl.LightningModule, float], None]
.Use case: attr-defined
This is the case that is described currently in the docs
Use case: partial optimizer
This is the case that is not compatible with the current API.
With this implementation it is not possible to use the
LearningRateFinder
callback orTuner
because the learning rate is not accessible through an attribute or the hyper parameters.Alternatives
I already implemented a version that achieves exactly that. The update is minimal and core changes will be in the
_lr_find
function, fromlightning/pytorch/tuner/lr_finder.py
module.There are only two places to update to make this work:
attr_name
, since this new feature will use alr_setter
function. We could add something similar that automatically generates alr_setter
if none is provided: first check if alr
orlearning_rate
attr is defined and create associated setter, then check if there is a partial optimizer defined namedoptim
oroptimizer
and adapt thelr_setter
. Maybe not necessary, or force the user to specify the setter function.lr_setter
instead of using thelightning_setattr
function.Also this feature requires to rename the
attr_name
tolr_setter
to make it obvious that the parameter is a setter.Additional context
This feature will change the API and is not backward compatible, if the name
attr_name
is changed. However, the capacities remain the same, but offer more customization.I already have a working implementation of this feature, compatible with the latest version of lightning. If this is of interest, I can submit a PR.
Thanks, really loving this library!
cc @Borda
The text was updated successfully, but these errors were encountered: