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

Trainer.scale_batch_size requires model.batch_size instead of model.hparams.batch_size #2484

Closed
wietsedv opened this issue Jul 3, 2020 · 7 comments · Fixed by #3043
Closed
Labels
bug Something isn't working good first issue Good for newcomers help wanted Open to be worked on

Comments

@wietsedv
Copy link

wietsedv commented Jul 3, 2020

🐛 Bug

Trainer.scale_batch_size only works if a model has the batch_size property and does not work with model.hparams.batch_size even though all documentation points to the reverse.

To Reproduce

All of my hyperparameters are available as model.hparams like suggested in the documentation: (hyperparameters, option 3.
This means that my batch_size is available as model.hparams.batch_size.

This should be fully compatible with the documented example code of Trainer.scale_batch_size() since that code also uses model.hparams.batch_size instead of model.batch_size.

However, when I put my model in Trainer.scale_batch_size, I get the following error:

pytorch_lightning.utilities.exceptions.MisconfigurationException: Field batch_size not found in `model.hparams`

Example code

class LitModel(pl.LightningModule):
    def __init__(self, hparams):
        super().__init__()
        self.hparams = args

model = LitModel(args)
trainer = Trainer()
trainer.scale_batch_size(model)

Expected behavior

Either Trainer.scale_batch_size should work with model.hparams or the error message, linked documentation examples and docstrings should all change (i.e. here, here and here).

(I would prefer the second option. I think that it should work with both model.batch_size and model.hparams.batch_size.)

Environment

  • pytorch-lightning 0.8.4
@wietsedv wietsedv added bug Something isn't working help wanted Open to be worked on labels Jul 3, 2020
@github-actions
Copy link
Contributor

github-actions bot commented Jul 3, 2020

Hi! thanks for your contribution!, great first issue!

@Borda Borda added duplicate This issue or pull request already exists good first issue Good for newcomers and removed duplicate This issue or pull request already exists labels Jul 3, 2020
@Borda
Copy link
Member

Borda commented Jul 3, 2020

it seems like a nice first issue, @wietsedv mind send a PR? 🐰

@MilesCranmer
Copy link

Appears to be the same with the learning rate parameter.

@ghost ghost mentioned this issue Jul 6, 2020
@remisphere
Copy link

A clean fix on the user side while waiting for the PR is to actually use self.hparams.batch_size and define self.batch_size as a property of your module:

@property
def batch_size(self):
    return self.hparams.batch_size

@batch_size.setter
def batch_size(self, batch_size):
    self.hparams.batch_size = batch_size

That way you keep your hyper parameters together in case you want to dump them somewhere without having to add specific code.

@remisphere
Copy link

remisphere commented Jul 24, 2020

From #1896 it seems that the problem is rather on the docs side than scale_batch_size()'s implementation.
self.batch_size is the correct location to look for the parameter, not self.hparams.batch_size.
My above fix is thus also obsolete.

@fepegar
Copy link
Contributor

fepegar commented Jul 24, 2020

From #1896 it seems that the problem is rather on the docs side than scale_batch_size()'s implementation.
My above fix is thus also obsolete.

I just tried your fix and it seemed to work :)

@remisphere
Copy link

Yes it does work, but from what they said in the PR I linked, hparams was just there as a temporary solution, and all hyper parameters are intended to be set as direct instance attributes in __init__.
My fix is obsolete regarding the intended usage of PL.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working good first issue Good for newcomers help wanted Open to be worked on
Projects
None yet
5 participants