Skip to content
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

prune Tune arguments from Trainer init - LearnRate & BatchSize as callback #9103

Closed
Borda opened this issue Aug 25, 2021 · 8 comments · Fixed by #16462
Closed

prune Tune arguments from Trainer init - LearnRate & BatchSize as callback #9103

Borda opened this issue Aug 25, 2021 · 8 comments · Fixed by #16462
Assignees
Labels
design Includes a design discussion discussion In a discussion stage feature Is an improvement or enhancement help wanted Open to be worked on priority: 1 Medium priority task tuner

Comments

@Borda
Copy link
Member

Borda commented Aug 25, 2021

🚀 Feature

Move the auto_scale_batch_size and auto_lr_find to the .tune method

Motivation

it is quite confusing setting some auto finds which needs to be called anyway with .tune() which uses another set of arguments.

Pitch

As a naive user, I would expect that the tuning is executed at the begging of .fit()

Alternatives

Also in such cases, the user is not much under control in which order they are called, for example, my use case is:

  1. find the largest batch size
  2. do something extra around the model
  3. find the best LR

ad this moment I would need to create two Traner instances :(

I am also experiencing some OOM memory failer when I run Batch size and LR, have not the concrete case for debugging yet

Additional context

The only justification would be usage with Lightning CLI

@Borda Borda added feature Is an improvement or enhancement help wanted Open to be worked on discussion In a discussion stage Important design Includes a design discussion tuner labels Aug 25, 2021
@ananthsub
Copy link
Contributor

I see 3 options:

  • keep as is
  • move constructor arguments to specific method they apply to (applies to tune, fit, validate, test, predict as well)
  • create separate objects for users to interact with: Trainer, Evaluator, Predictor, Tester

the last one is the most extreme, but i wanted to mention for completeness. between the first two, i prefer the second one. there's a similar conversation here: #9006

@SkafteNicki
Copy link
Member

Additional option:
Move lr finder and batch scaler to callbacks, removing the tune method
auto_scale_batch_size and auto_lr_find could still exist, setting them to True would just add the corresponding callback.

@Borda
Copy link
Member Author

Borda commented Aug 25, 2021

Move lr finder and batch scaler to callbacks, removing the tune method

I like this one and it may be the cleanest way as we don't add any other logic pattern/layer

@ananthsub
Copy link
Contributor

@SkafteNicki @Borda - as a callback, this can be pretty invasive in setting data inside of the trainer. Is that a precedent we should be setting? In what callback hooks would these run? https://github.com/PyTorchLightning/pytorch-lightning/blob/69f66fd6bb361a7932a82291e4ef001f4f381f99/pytorch_lightning/tuner/batch_size_scaling.py#L115-L125

@Borda
Copy link
Member Author

Borda commented Aug 26, 2021

I assume that all would run in the before fit hook...

@tchaton
Copy link
Contributor

tchaton commented Aug 26, 2021

IMO, I trust each entrypoint to the trainer to be independent as it makes sure features are fully encapsultated and reduce risks from bugs.

move constructor arguments to specific method they apply to (applies to tune, fit, validate, test, predict as well)

I think this is fine for tune which is pretty particular, but It don't think we should do this for other functions.

Best,
T.C

@SkafteNicki
Copy link
Member

I am also fine with moving the arguments to the tune method.
Tuning is the odd one out of tune, fit, validate, test, predict so I think we can do it without having to move other arguments.

@Borda Borda changed the title remove Tune arguments from Trainer init prune Tune arguments from Trainer init - LearnRate & BatchSize as callback Sep 30, 2021
@Borda
Copy link
Member Author

Borda commented Oct 15, 2021

I model practical use-case is combination with FineTune which would unfreeze backbone on 10th epoch
so on:

  • 0.epoch - freeze backbone
  • 0.epoch - run BatchSize -> 45
  • 0.epoch - run LRFinder -> 0.005
  • 9.epoch - unfreeze backbone
  • 9.epoch - run BatchSize -> 14
  • 9.epoch - run LRFinder -> 0.001

this gives more freedom to combine these methods during the training cycle
so the callback will be executed on epoch start...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
design Includes a design discussion discussion In a discussion stage feature Is an improvement or enhancement help wanted Open to be worked on priority: 1 Medium priority task tuner
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants