Deprecate @auto_move_data in favor of trainer.predict

[carmocca]

What does this PR do?

See title.

This was added in 0.8.0.

A quick code search shows some users of this feature, but many don't use it correctly or assume it's required. Additionally, we want people to use predict.

Also improves predict docs.

Before submitting

• Was this discussed/approved via a GitHub issue? (not for typos and docs)
• Did you read the contributor guideline, Pull Request section?
• Did you make sure your PR does only one thing, instead of bundling different changes together?
• Did you make sure to update the documentation with your changes? (if necessary)
• Did you write any new necessary tests? (not for typos and docs)
• Did you verify new and existing tests pass locally with your changes?
• Did you update the CHANGELOG? (not for typos, docs, test updates, or internal minor changes/refactorings)

PR review

• Is this pull request ready for review? (if not, please submit in draft mode)
• Check that all items from Before submitting are resolved
• Make sure the title is self-explanatory and the description concisely explains the PR
• Add labels and milestones (and optionally projects) to the PR so it can be classified

[codecov]

Codecov Report

Merging #6993 (8a99dd5) into master (e891ceb) will decrease coverage by 9%.
The diff coverage is 100%.

@@           Coverage Diff            @@
##           master   #6993     +/-   ##
========================================
- Coverage      92%     83%     -9%     
========================================
  Files         194     194             
  Lines       12328   13236    +908     
========================================
- Hits        11324   10993    -331     
- Misses       1004    2243   +1239     

[dsantiago]

I am finding some trouble with Trainer.predict(). I am trying this:

#this
trainer.predict(model, target)

# and this:
model.eval()
trainer.predict(model, target)

But even with eval() i am getting BatchNorm error ... So the predict is running Dropouts and BatchNorm layers... in the end as my target dataset is small i am the follow as they work as should:

# this
def predictions(model, inputs):
  model.eval()
  with torch.no_grad():
    preds = model(inputs)
  
  model.train()
  return preds    

predictions(model, test_data)

# or this
model.predict(test_data, batch_idx=1) # as my model is from pl.lightining module, batch_id can be whatever

The question is, why Trainer.predict() do not use the model.eval() + torch.no_grad() approach ?

Just to note, in my pl model i just implemented the def training_step function, no valid_step or test_step, but i still think that validation and test/predict functions should use model.eval() + torch.no_grad()

[carmocca]

I am finding some trouble with Trainer.predict(). I am trying this: ...

Hi @dsantiago, can you open a separate issue about this? A reproduction snippet would also be greatly appreciated.

[dsantiago]

Hi @carmocca it's just a point, not an error, i am just asking if is it OK for Trainer.predict() run Dropout and BatchNorm layers, because for me it's counter intuitive.... But i can always rely in calling model directly, with model.eval() + torch.nograd() or use the pytorch lightning model's predict() function...

It's just to reflect...

[carmocca]

it's just a point, not an error

It should be getting called:

https://github.com/PyTorchLightning/pytorch-lightning/blob/4c07ab5e99dd20c1f309d9e73cdaacc1ebad9499/pytorch_lightning/trainer/trainer.py#L797-L800

So if it is not, it's an error.

So please, try to reproduce it and open an issue about it 😃

[dsantiago]

it's just a point, not an error

It should be getting called:

https://github.com/PyTorchLightning/pytorch-lightning/blob/4c07ab5e99dd20c1f309d9e73cdaacc1ebad9499/pytorch_lightning/trainer/trainer.py#L797-L800

So if it is not, it's an error.

So please, try to reproduce it and open an issue about it 😃

I saw that part of the code when trying to understand, and it really just disable the grad calculation, if the model isn't in eval mode, it will continue to run Dropouts and BatchNorm layers...

Just one call to model.eval() before getting the model predictions and put it on model.train() back after should make de deal(like in the predictions() function i did above).

[carmocca]

on_predict_mode_eval calls model.eval()

[dsantiago]

on_predict_mode_eval calls model.eval()

Well i removed some boilerplate code so you can verify if u wish, dont need to add gpus as the network is kind of small. Maybe i am missing the point from my side

https://tinyurl.com/43cjas52

[awaelchli]

I know what the problem is with your code @dsantiago, but let's not discuss that here. Please open a discussion (discussion tab on top next to Pull Requests) ping me there.

[awaelchli]

😢 😢 😢

[dsantiago]

I know what the problem is with your code @dsantiago, but let's not discuss that here. Please open a discussion (discussion tab on top next to Pull Requests) ping me there.

Thanks @awaelchli , did it now! Thanks for your time in seeing it!

[tchaton]

LGTM !