bug with _load_optimizer_and_scheduler in trainer.py
Author: rabeehk
Pull request: no

## Environment info
- `transformers` version: 3.5.1
- Platform: GPU
- Python version: 3.7
- PyTorch version (GPU?): 1.4.0
- Using GPU in script?: No
- Using distributed or parallel set-up in script?: No

### Who can help
 Trainer: @sgugger
 Text Generation: @patrickvonplaten

## Information
I am using finetune_trainer.py, what I am observing is that in trainer.py, when you call _load_optimizer_and_scheduler if the model_path folder exist, then this ignores the user's set learning rate, meaning it continues with some saved learning rate than actually use the learning rate user set, could you have a look please? thanks 


Comments:
Hi @rabeehk ,

In `_load_optimizer_and_scheduler` if `model_path` exists and `optimizer` and `scheduler` `state_dict` is found then that means you are loading from a saved checkpoint and continue training from there,  so the `lr` is read from the `scheduler` and used instead of the set LR. This is expected behaviour.

Hi Suraj,
thanks for the reply. I have a couple of questions on this, 1) I see this
is ignoring the training epochs in when loading from the saved checkpoints,
so it does not train for the epochs set, how could I resolve it? Also, if I
want to change the lr, could I load from checkpoint but change the lr?could
you give me some information how loading from trained optimizer, could
help?

to explain better, I train a model for X epochs, then I want to finetune it
on other datasets with extra Y epochs with different learning rate, for
this I pass the updated model to trainer, but then should I pass the
model_path so it loads from the saved checkpoint of optimizer? and why this
is ignoring the set number of epochs?
thanks

On Mon, Dec 14, 2020 at 11:07 AM Suraj Patil <notifications@github.com>
wrote:

> Hi @rabeehk <https://github.com/rabeehk> ,
>
> In _load_optimizer_and_scheduler if model_path exists and optimizer and
> scheduler state_dict is found then that means you are loading from a
> saved checkpoint and continue training from there, so the lr is read from
> the scheduler and used instead of the set LR. This is expected behaviour.
>
> —
> You are receiving this because you were mentioned.
> Reply to this email directly, view it on GitHub
> <https://github.com/huggingface/transformers/issues/9099#issuecomment-744366176>,
> or unsubscribe
> <https://github.com/notifications/unsubscribe-auth/ABP4ZCDDDMJGNROAD3AKZFTSUXWVJANCNFSM4U2SCXGA>
> .
>


If you want to fine-tune the saved checkpoint on another dataset then you could save it in diff path or remove the saved `optimizer` and `scheduler` files.

Also @sgugger  might have a better answer here.

Hi Suraj,

I am having a similar problem here.
When the trainer continues from a checkpoint, i.e. `trainer.train(check_point_path)`, I notice a peek in the learning curve. I suspect that is related to what Rabeeh has mentioned.

Please have a look at the learning curve I got after I had to resume the training twice.

![image](https://user-images.githubusercontent.com/5329543/102083242-4e663d00-3e24-11eb-93b8-93d4e27a6061.png)

Any ideas?

> this I pass the updated model to trainer, but then should I pass the model_path so it loads from the saved checkpoint of optimizer? and why this is ignoring the set number of epochs?

Passing a `model_path` to the train method is done when you want to resume an interrupted training, which is why it does not do all epochs (it resumes the training from where you where). If you want to do a new training, you should not use that argument and manually pass the optimizer/scheduler you want to use at init.

@abdullah-alnahas I have no idea what your plot is since you haven't told us how you generated it. 

Thanks for your response @sgugger , and sorry for not making myself clear.

I am training an Electra model from scratch using the [`Trainer`] API.(https://huggingface.co/transformers/main_classes/trainer.html). I have interrupted the trainer twice, then resumed training by `trainer.train(latest_checkpoint_path)`.
After that, I have generated the learning curve plot from `{latest_checkpoint_path}/trainer_state.json`'s  `log_history` using `step` as the x axis, and `loss` as the y axis.

My question: Is it normal that the learning curve peaks after resuming the training from a checkpoint after an interruption?

The loss is reinitialized to 0 (it's not saved with the checkpoints) so it could come from this. There were also some recent changes in how the loss is logged so having your transformers version would help. The CI tests the final values of the weights of a (small) model are the same with a full training or resumed training, so I think this is just some weird reporting of the loss.

thanks Suraj and everyone, makes sense not to initialize the optimizers.

Hi @sgugger, 

I encountered the same issue on Transformers 4.3.0. I think the problem is not the loss being reinitialized as 0, but that the model is not being loaded from model_path. Only `TrainerState` is loaded but not the model weights. I looked through the code before concluding this, but as a sanity check, the current code will run even if `pytorch_model.bin` is not in the checkpoint directory, confirming that its not being loaded at all. It's odd that the CI tests are passing....

Anyway I modified `trainer.py:train()`under the code block:

```
# Check if continuing training from a checkpoint
if model_path and os.path.isfile(os.path.join(model_path, "trainer_state.json")):
...
  self._globalstep_last_logged = self.state.global_step 

  if isinstance(self.model, PreTrainedModel):
    model = model.from_pretrained(model_path)
  if not self.is_model_parallel:
    model = model.to(self.args.device)
  else:
    state_dict = torch.load(os.path.join(model_path, WEIGHTS_NAME))
    model.load_state_dict(state_dict)
```

`self._globalstep_last_logged = self.state.global_step` ensures the first logging of the loss is correct. `self._globalstep_last_logged` should not be 0 (that line is removed in the later part of the code)

The training is properly resumed after this. 



`Trainer` does not handle the reloading of the model indeed, which can be confusing. So l'll add that functionality this afternoon!

This issue has been automatically marked as stale because it has not had recent activity. If you think this still needs to be addressed please comment on this thread.

Please note that issues that do not follow the [contributing guidelines](https://github.com/huggingface/transformers/blob/master/CONTRIBUTING.md) are likely to be ignored.