Repository: pytorch/pytorch
Pull request: _LRSchedulers getstate include optimizer info
This PR fixes #7255.
`__getstate__` now have different behavior with state_dict() since it should include optimizer information. 

Problem statement:
torch.save does not work with _LRSchedulers
## Issue description

``torch.save`` does not work with ``torch.optim.lr_scheduler._LRScheduler``

## Code example

Offending code:
```
from torch import nn
from torch.optim import Adam
from torch.optim.lr_scheduler import MultiStepLR

import torch

net = nn.LSTM(10, 10)
optimizer = Adam(params=filter(lambda p: p.requires_grad, net.parameters()))
scheduler = MultiStepLR(optimizer, milestones=[30, 80], gamma=0.1)

scheduler.step()  # This is Fine

# Check save and load into the directory work
torch.save(scheduler, 'scheduler.pt')
scheduler = torch.load('scheduler.pt')

scheduler.step()  # AttributeError: 'MultiStepLR' object has no attribute 'optimizer'
```

Stack trace:
```
Traceback (most recent call last):
  File "play.py", line 17, in <module>
    scheduler.step()  # AttributeError: 'MultiStepLR' object has no attribute 'optimizer'
  File "/usr/local/lib/python3.6/dist-packages/torch/optim/lr_scheduler.py", line 55, in step
    for param_group, lr in zip(self.optimizer.param_groups, self.get_lr()):
AttributeError: 'MultiStepLR' object has no attribute 'optimizer'
````

## System Info
- PyTorch or Caffe2: PyTorch
- How you installed PyTorch (conda, pip, source): pip
- PyTorch version: 0.4.0
- Python version: 3.6.4

Hints from issue discussion:
This is due to _LRScheduler.state_dict explicitly excluding optimizer in state_dict(). https://github.com/pytorch/pytorch/blob/master/torch/optim/lr_scheduler.py#L37 Including it in state_dict() fixes the problem. Before I go ahead and make the PR, is there any other concern that we should not put optimizer in state_dict()? cc: @apaszke Thanks!
Yeah, `__getstate__` and `state_dict()` should really have different logic (with the first one including optimizer, while the second one would not).
Is including the optimizer a good idea though? I can imagine a code like this 

```python
optimizer = th.load(...)
scheduler = th.load(...)

scheduler.step() # this will not change lr of the loaded optimizer, but of the internal copy?
```

The same thing happens if you serialize a model and its optimizer in two separate checkpoints, so I'm not very concerned about that.

Commit message: getstate should include optimizer

Patch to review:
diff --git a/torch/optim/lr_scheduler.py b/torch/optim/lr_scheduler.py
index f1bef0d84b73..352561d3d779 100644
--- a/torch/optim/lr_scheduler.py
+++ b/torch/optim/lr_scheduler.py
@@ -23,7 +23,7 @@ def __init__(self, optimizer, last_epoch=-1):
         self.last_epoch = last_epoch
 
     def __getstate__(self):
-        return self.state_dict()
+        return self.__dict__
 
     def __setstate__(self, state):
         self.load_state_dict(state)


Code review comments:
diff_hunk: @@ -23,7 +23,7 @@ def __init__(self, optimizer, last_epoch=-1):
         self.last_epoch = last_epoch
 
     def __getstate__(self):
-        return self.state_dict()
+        return self.__dict__
 
     def __setstate__(self, state):
         self.load_state_dict(state)
path: torch/optim/lr_scheduler.py
text: @user1:
Can you just remove `__getstate__` and `__setstate__`. They should really do the default thing here