Refer warmup_ratio when setting warmup_num_steps.
Author: tsuchm
Pull request: yes

# What does this PR do?

This PR fixes the bug that DeepSpeed schduler does not refer warmup_ratio argument, although DeepSpeed schduler refers warmup_steps argument.   This contradicts to the warning message "Both warmup_ratio and warmup_steps given, warmup_steps will override any effect of warmup_ratio during training" defined in training_args.py L.700.

## Before submitting
- [ ] This PR fixes a typo or improves the docs (you can dismiss the other checks if that's the case).
- [x] Did you read the [contributor guideline](https://github.com/huggingface/transformers/blob/master/CONTRIBUTING.md#start-contributing-pull-requests),
      Pull Request section?
- [ ] Was this discussed/approved via a Github issue or the [forum](https://discuss.huggingface.co/)? Please add a link
      to it if that's the case.
- [ ] Did you make sure to update the documentation with your changes? Here are the
      [documentation guidelines](https://github.com/huggingface/transformers/tree/master/docs), and
      [here are tips on formatting docstrings](https://github.com/huggingface/transformers/tree/master/docs#writing-source-documentation).
- [ ] Did you write any new necessary tests?

## Who can review?

Because this PR relates to DeepSpeed integration, I hope @stas00 may review this.

Comments:
Thank you for your PR, @tsuchm 

I'm trying to understand what you're trying to accomplish and could use some help from you.

Did I understand you correctly that you're saying that Deepspeed integration ignores the `--warmup_ratio` argument and you're proposing how to integrate it? I agree that it could be integrated.

In which case the logic should be the same as in:

https://github.com/huggingface/transformers/blob/cabcc75171650f9131a4cf31c62e1f102589014e/src/transformers/trainer.py#L825-L828

so that `--warmup_steps` takes precedence and we are matching HF Trainer consistently.

The part that I didn't get is what does this have to do with the warning from:

https://github.com/huggingface/transformers/blob/cabcc75171650f9131a4cf31c62e1f102589014e/src/transformers/training_args.py#L724-L727

Thank you.

> Did I understand you correctly that you're saying that Deepspeed integration ignores the --warmup_ratio argument and you're proposing how to integrate it? I agree that it could be integrated.

Yes, your understanding is correct.

> In which case the logic should be the same as in:

Yes, my PR also aimed to reproduce your pointed code, and I agree that my PR is not exactly equal to your pointed code.
I however think that if you, the development team, hope that these two fragments are exactly same, it is better to abstract them into a function than to copy it.  Unfortunately, I do not have enough knowledge to abstract them.

> The part that I didn't get is what does this have to do with the warning from:

I am sorry for my confusing comment.  My aim to quote the above warning is simply to indicate that this behavior (DeepSpeed scheduler does not refer warmup_ratio argument) does not match the action that we users expect.   I think that no fix for this warning message is necessary. 

I totally agree on all accounts. Thank you for the clarification. 

Would you like me to push the changes into your PR branch or would you rather try to do it yourself with my guidance? 

Basically this code:  
```
warmup_steps = ( 
     self.args.warmup_steps 
     if self.args.warmup_steps > 0 
     else math.ceil(num_training_steps * self.args.warmup_ratio)
```
can be made into a method around the same place, e.g. `def get_warmup_steps(self, num_training_steps)`

and then we can call it in both places, in the same way.

Let me know what you prefer.

p.s. may be it best belongs  in `TrainingArguments` - let's ask @sgugger where he prefers such wrapper to reside. but for now edit it in place as I suggested if you'd like to try to work it out by yourself.
     

I agree your opinion.   I tried to add a new method to get number of warmup steps to TrainingArguments class.  Unfortunately, my using environment uses the version 4.8.2, thus, I confirmed my proposal works on the version 4.8.2, and backported it to the main branch.

Could you review my PR?

a gentle note to reviewers: please don't forget the docs.

Fixed here: https://github.com/huggingface/transformers/pull/12830

I'm sorry I missed the documentation inside `deepsped`, the main point of the PR was that it was documented a certain way in `training_args.py` and not enforced.

That's alright, @sgugger. I was just asleep to have a chance to review.

My understanding is that the main intention of this PR was to fix the missing functionality in the Deepspeed integration. And in the process other parts of the ecosystem were touched.

Thanks to all reviewers.   Finally, I have just succeeded to reproduce results of a previous research using warmup strategy.