Fix Adafactor documentation (recommend correct settings)

[jsrozner]

This PR fixes documentation to reflect optimal settings for Adafactor:

• fix an impossible arg combination erroneously proposed in the example
• use the correct link to the adafactor paper where clip_treshold is discussed
• document the recommended scale_parameter=False
• add other recommended settings combinations, which are quite different from the original
• re-org notes
• make the errors less ambiguous

Fixes #7789

@sgugger

(edited by @stas00 to reflect it's pre-merge state as the PR evolved since it's original submission)

[jsrozner]

Is this part correct?

Recommended T5 finetuning settings:
    - Scheduled LR warm-up to fixed LR
    - disable relative updates
    - use clip threshold: https://arxiv.org/abs/2004.14546

In particular:

• are we supposed to do scheduled LR? adafactor handles this no?
• we should not disable relative updates
• i don't know what clip threshold means in this context

[jsrozner]

@sshleifer can you accept this documentation change?

[sshleifer]

No but @stas00 can!

[stas00]

@jsrozner, thank you for the PR.

Reading #7789 it appears that the Recommended T5 finetuning settings are invalid.

So if we are fixing this, in addition to changing the example the prose above it should be synced as well.

I don't know where the original recommendation came from - do you by chance have a source we could point to for the corrected recommendation? If you know that is, if not, please don't worry.

Thank you.

[avacaondata]

I receive the following error when using this the "recommended way":

Traceback (most recent call last):
  File "./folder_aws/transformers/examples/seq2seq/run_seq2seq.py", line 759, in <module>
    main()
  File "./folder_aws/transformers/examples/seq2seq/run_seq2seq.py", line 651, in main
    train_result = trainer.train(resume_from_checkpoint=checkpoint)
  File "/home/alejandro.vaca/SpainAI_Hackaton_2020/folder_aws/transformers/src/transformers/trainer.py", line 909, in train
    self.create_optimizer_and_scheduler(num_training_steps=max_steps)
  File "/home/alejandro.vaca/SpainAI_Hackaton_2020/folder_aws/transformers/src/transformers/trainer.py", line 660, in create_optimizer_and_scheduler
    self.optimizer = optimizer_cls(optimizer_grouped_parameters, **optimizer_kwargs)
  File "/home/alejandro.vaca/SpainAI_Hackaton_2020/folder_aws/transformers/src/transformers/optimization.py", line 452, in __init__
    raise ValueError("warmup_init requires relative_step=True")
ValueError: warmup_init requires relative_step=True

Following this example in documentation:

Adafactor(model.parameters(), lr=1e-3, relative_step=False, warmup_init=True)

@sshleifer @stas00 @jsrozner

[stas00]

@alexvaca0, what was the command line you run when you received this error?

HF Trainer doesn't set warmup_init=True. Unless you modified the script?

transformers/src/transformers/trainer.py

Lines 649 to 651 in 21e86f9

	if self.args.adafactor:
	optimizer_cls = Adafactor
	optimizer_kwargs = {"scale_parameter": False, "relative_step": False}

It is possible that the whole conflict comes from misunderstanding how this optimizer has to be used?

To use a manual (external) learning rate schedule you should set scale_parameter=False and relative_step=False.

which is what the Trainer does at the moment.

and:

   relative_step (:obj:`bool`, `optional`, defaults to :obj:`True`):
       If True, time-dependent learning rate is computed instead of external learning rate

Is it possible that you are trying to use both an external and the internal scheduler at the same time?

It'd help a lot of you could show us the code that breaks, (perhaps on colab?) and how you invoke it. Basically, help us reproduce it.

Thank you.

[avacaondata]

Hi, @stas00 , first thank you very much for looking into it so fast. I forgot to say it, but yes, I changed the code in Trainer because I was trying to use the recommended settings for training T5 (I mean, setting an external learning rate with warmup_init = True as in the documentation. Training without LR warmup or clip threshold is not recommended. Additional optimizer operations like gradient clipping should not be used alongside Adafactor. From your answer, I understand that Trainer was designed for using Adam, as it uses the external learning rate scheduler and doesn't let you pass None as learning rate. Is there a workaround to be able to use the Trainer class with Adafactor following Adafactor settings recommended in the documentation? I'd also like to try using Adafactor without specifying the learning rate, would that be possible? I think maybe this documentation causes a little bit of confusion, because when you set the parameters specified in it Adafactor(model.parameters(), lr=1e-3, relative_step=False, warmup_init=True) it breaks.

[stas00]

OK, so first, as @jsrozner PR goes and others commented, the current recommendation appears to be invalid.

So we want to gather all the different combinations that work and identify which of them provides the best outcome. I originally replied to this PR asking if someone knows of an authoritative paper we could copy the recommendation from - i.e. finding someone who already did the quality studies, so that we won't have it. So I'm all ears if any of you knows of such source.

Now to your commentary, @alexvaca0, while HF trainer has Adam as the default it has --adafactor which enables it in the mode with using the external scheduler. Surely, we could change the Trainer to skip the external scheduler (or perhaps simpler feeding it some no-op scheduler) and instead use this recommendation if @sgugger agrees with that. But we first need to see that it provides a better outcome in the general case. Or alternatively to make --adafactor configurable so it could support more than just one way.

For me personally I want to understand first the different combinations, what are the impacts and how many of those combinations should we expose through the Trainer. e.g. like apex optimization level, we could have named combos --adafactor setup1, --adafactor setup2 and would activate the corresponding configuration. But first let's compile the list of desirable combos.

Would any of the current participants be interested in taking a lead on that? I'm asking you since you are already trying to get the best outcome with your data and so are best positioned to judge which combinations work the best for what situation.

Once we compiled the data it'd be trivial to update the documented recommendation and potentially extend HF Trainer to support more than one setting for Adafactor.

[sgugger]

I only ported the --adafactor option s it was implemented for the Seq2SeqTrainer to keep the commands using it working as they were. The Trainer does not have vocation to support all the optimizers and all their possible setups, just one sensible default that works well, that is the reason you can:

• pass an optimizer at init
• subclass and override create_optimizer.

In retrospect, I shouldn't have ported the Adafactor option and it should have stayed just in the script using it.

[stas00]

Thank you for your feedback, @sgugger.

So let's leave the trainer as it is and let's then solve this for Adafactor as just an optimizer and then document the best combinations.

[jsrozner]

Per my comment on #7789, I observed that

I can confirm that Adafactor(lr=1e-3, relative_step=False, warmup_init=False) seems to break training (i.e. I observe no learning over 4 epochs, whereas Adafactor(model.parameters(), relative_step=True, warmup_init=True, lr=None) works well (much better than adam)

Given that relative_step and warmup_init must take on the same value, it seems like there is only one configuration that is working?

But, this is also confusing (see my comment above): #10526 (comment)

Recommended T5 finetuning settings:
    - Scheduled LR warm-up to fixed LR
    - disable relative updates
    - use clip threshold: https://arxiv.org/abs/2004.14546

In particular:

• are we supposed to do scheduled LR? adafactor handles this no?
• we should not disable relative updates
• i don't know what clip threshold means in this context

[stas00]

I first validated that HF Adafactor is 100% identical to the latest fairseq version.

I then tried to find out the source of these recommendations and found:

1. https://discuss.huggingface.co/t/t5-finetuning-tips/684/3

lr=0.001, scale_parameter=False, relative_step=False

2. https://discuss.huggingface.co/t/t5-finetuning-tips/684/22 which is your comment @jsrozner where you propose that the opposite combination works well:

lr=None, relative_step=True, warmup_init=True

If both found to be working, I propose we solve this conundrum by documenting this as following:

     Recommended T5 finetuning settings (https://discuss.huggingface.co/t/t5-finetuning-tips/684/3):

        - Scheduled LR warm-up to fixed LR
        - disable relative updates
        - scale_parameter=False
        - use clip threshold: https://arxiv.org/abs/2004.14546

        Example::

            Adafactor(model.parameters(), scale_parameter=False, relative_step=False, warmup_init=False, lr=1e-3)

    Others reported the following combination to work well::

        Adafactor(model.parameters(), scale_parameter=False, relative_step=True, warmup_init=True, lr=None)

        - Training without LR warmup or clip threshold is not recommended. Additional optimizer operations like
          gradient clipping should not be used alongside Adafactor.

https://discuss.huggingface.co/t/t5-finetuning-tips/684/22 Also highly recommends to turn scale_parameter=False - so I added that to the documentation and the example above in both cases. Please correct me if I'm wrong.

And @jsrozner's correction in this PR is absolutely right to the point.

Please let me know if my proposal makes sense, in particular I'd like your validation, @jsrozner, since I added your alternative proposal. And don't have any other voices to agree or disagree with it.

Thank you!

[stas00]

Is this part correct?

Recommended T5 finetuning settings:
    - Scheduled LR warm-up to fixed LR
    - disable relative updates
    - use clip threshold: https://arxiv.org/abs/2004.14546

In particular:

* are we supposed to do scheduled LR? adafactor handles this no?

see my last comment - it depends on whether we use the external LR scheduler or not.

* we _should not_ disable relative updates

see my last comment - it depends on warmup_init's value

* i don't know what clip threshold means in this context

this?

    def __init__(
       [....]
        clip_threshold=1.0,

[avacaondata]

I'm running some experiments, playing around with Adafactor parameters. I'll post here which configuration has best results. From T5 paper, they used the following parameters for fine-tuning: Adafactor with constant lr 1e-3, with batch size 128, if I understood the paper well. Therefore, I find it appropriate the documentation changes mentioned above, leaving the recommendations from the paper while mentioning other configs that have worked well for other users. In my case, for example, the configuration from the paper doesn't work very well and I quickly overfit.

[stas00]

Finally, I'm trying to understand the confusing:

        - use clip threshold: https://arxiv.org/abs/2004.14546
        [...]
        gradient clipping should not be used alongside Adafactor.

As the paper explains these are 2 different types of clipping.

Since the code is:

update.div_((self._rms(update) / group["clip_threshold"]).clamp_(min=1.0))

this probably means that the default clip_threshold=1.0 is in effect disables clip threshold.

I can't find any mentioning of clip threshold in https://arxiv.org/abs/2004.14546 - is this a wrong paper? Perhaps it needed to link to the original paper https://arxiv.org/abs/1804.04235 where clipping is actually discussed? I think it's the param d in the latter paper and it proposes to get the best results with d=1.0 without learning rate warmup:

page 5 from https://arxiv.org/pdf/1804.04235:

We added update clipping to the previously described fast-
decay experiments. For the experiment without learning rate
warmup, update clipping with d = 1 significantly amelio-
rated the instability problem – see Table 2 (A) vs. (H). With
d = 2, the instability was not improved. Update clipping
did not significantly affect the experiments with warmup
(with no instability problems).

So I will change the doc to a non-ambiguous:

use clip_threshold=1.0

[stas00]

OK, so here is the latest proposal. I re-organized the notes:

    Recommended T5 finetuning settings (https://discuss.huggingface.co/t/t5-finetuning-tips/684/3):

        - Training without LR warmup or clip_threshold is not recommended. 
           * use scheduled LR warm-up to fixed LR
           * use clip_threshold=1.0 (https://arxiv.org/abs/1804.04235)
        - Disable relative updates
        - Use scale_parameter=False
        - Additional optimizer operations like gradient clipping should not be used alongside Adafactor

        Example::

            Adafactor(model.parameters(), scale_parameter=False, relative_step=False, warmup_init=False, lr=1e-3)

        Others reported the following combination to work well::

            Adafactor(model.parameters(), scale_parameter=False, relative_step=True, warmup_init=True, lr=None)

I added these into this PR, please have a look.

[sgugger]

Just two nits to highlight the parameters in the error message, thanks for fixing!

[stas00]

Let's just wait to hear from both @jsrozner and @alexvaca0 to ensure that my edits are valid before merging.

[jsrozner]

I observed that Adafactor(lr=1e-3, relative_step=False, warmup_init=False) failed to lead to any learning. I guess this is because I didn't change scale_parameter to False? I can try rerunning with scale_param false.

And when I ran with Adafactor(model.parameters(), relative_step=True, warmup_init=True, lr=None), I did not set scale_parameter=False. Before adding the "others seem to have success with ..." bit, we should check on the effect of scale_parameter.

Regarding clip_threshold - just confirming that the comment is correct that when using adafactor we should not have any other gradient clipping (e.g. nn.utils.clip_grad_norm_())?

Semi-related per @alexvaca0, regarding T5 paper's recommended batch_size: is the 128 recommendation agnostic to the length of input sequences? Or is there a target number of tokens per batch that would be optimal? (E.g. input sequences of max length 10 tokens vs input sequences of max length 100 tokens -- should we expect 128 to work optimally for both?)

But most importantly, shouldn't we change the defaults so that a call to Adafactor(model.paramaters()) == Adafactor(model.parameters(), scale_parameter=False, relative_step=False, warmup_init=False, lr=1e-3)

i.e, we default to what we suggest?

[stas00]

I observed that Adafactor(lr=1e-3, relative_step=False, warmup_init=False) failed to lead to any learning. I guess this is because I didn't change scale_parameter to False? I can try rerunning with scale_param false.

Yes, please and thank you!

Regarding clip_threshold - just confirming that the comment is correct that when using adafactor we should not have any other gradient clipping (e.g. nn.utils.clip_grad_norm_())?

Thank you for validating this, @jsrozner

Is the current incarnation of the doc clear wrt this subject matter or should we add an explicit example?

One thing I'm concerned about is that the Trainer doesn't validate this and will happily run clip_grad_norm with Adafactor

Might we need to add to:

transformers/src/transformers/trainer.py

Lines 649 to 651 in 9f8fa4e

	if self.args.adafactor:
	optimizer_cls = Adafactor
	optimizer_kwargs = {"scale_parameter": False, "relative_step": False}

if self.args.max_grad_norm:
    raise ValueError("don't use max_grad_norm with adafactor")

But most importantly, shouldn't we change the defaults so that a call to Adafactor(model.paramaters()) == Adafactor(model.parameters(), scale_parameter=False, relative_step=False, warmup_init=False, lr=1e-3)

Since we copied the code verbatim from fairseq, it might be a good idea to keep the defaults the same? I'm not attached to either way. @sgugger what do you think?

edit: I don't think we can/should since it may break people's code that relies on the current defaults.

[jsrozner]

Regarding clip_threshold - just confirming that the comment is correct that when using adafactor we should not have any other gradient clipping (e.g. nn.utils.clip_grad_norm_())?

Thank you for validating this, @jsrozner

Sorry, I didn't validate this. I wanted to confirm with you all that this is correct.

But most importantly, shouldn't we change the defaults so that a call to Adafactor(model.paramaters()) == Adafactor(model.parameters(), scale_parameter=False, relative_step=False, warmup_init=False, lr=1e-3)

Since we copied the code verbatim from fairseq, it might be a good idea to keep the defaults the same? I'm not attached to either way. @sgugger what do you think?

Alternative is to not provide defaults for these values and force the user to read documentation and decide what he/she wants. Can provide the default implementation as well as Adafactor's recommended settings

[jsrozner]

I'm running some experiments, playing around with Adafactor parameters. I'll post here which configuration has best results. From T5 paper, they used the following parameters for fine-tuning: Adafactor with constant lr 1e-3, with batch size 128, if I understood the paper well. Therefore, I find it appropriate the documentation changes mentioned above, leaving the recommendations from the paper while mentioning other configs that have worked well for other users. In my case, for example, the configuration from the paper doesn't work very well and I quickly overfit.

@alexvaca0
What set of adafactor params did you find work well when you were finetuning t5?

[stas00]

Sorry, I didn't validate this. I wanted to confirm with you all that this is correct.

I meant validating as in reading over and checking that it makes sense. So all is good. Thank you for extra clarification so we were on the same page, @jsrozner

Alternative is to not provide defaults for these values and force the user to read documentation and decide what he/she wants. Can provide the default implementation as well as Adafactor's recommended settings

as I appended to my initial comment, this would be a breaking change. So if it's crucial that we do that, this would need to happen in the next major release.

[jsrozner]

Or maybe add a warning message that indicates that default params may not be optimal? It will be logged only a single time at optimizer init so not too annoying.

log.warning('Initializing Adafactor. If you are using default settings, it is recommended that you read the documentation to ensure that these are optimal for your use case.)

[stas00]

But we now changed it propose two different ways - which one is the recommended one? The one used by the Trainer?

Since it's pretty clear that there is more than one way, surely the user will find their way to the doc if they aren't happy with the results.

[jsrozner]

I ran my model under three different adafactor setups:

          optimizer = Adafactor(self.model.parameters(),
                              relative_step=True,
                              warmup_init=True)

optimizer = Adafactor(self.model.parameters(),
                              relative_step=True,
                              warmup_init=True,
                              scale_parameter=False)

        optimizer = Adafactor(self.model.parameters(),
                              lr=1e-3,
                              relative_step=False,
                              warmup_init=False,
                              scale_parameter=False)

I track exact match and NLL on the dev set. Epochs are tracked at the bottom. They start at 11 because of how I'm doing things. (i.e. x=11 => epoch=1)

Note that I'm training a t5-small model on 80,000 examples, so maybe there's some variability with the sort of model we're training?

(Image works if you navigate to the link, but seems not to appear?)

purple is (1)
blue is (2) and by far the worst (i.e. shows that scale_param should be set to True if we are using relative_step)
brown is (3)

In particular, it looks like scale_param should be True for the setting under " Others reported the following combination to work well::"

On the other hand, it looks like for a t5-large model, (3) does better than (1) (although I also had substantially different batch sizes).

[stas00]

Thank you, @jsrozner, for running these experiments and the analysis.

So basically we should adjust " Others reported the following combination to work well::" to scale_param=True, correct?

Interestingly we end up with 2 almost total opposites.

[avacaondata]

@jsrozner Batch size and learning rate configuration go hand in hand, therefore it's difficult to know about your last reflexion, as having different different batch sizes lead to different gradient estimations (in particular, the lower the batch size, the worse your gradient estimation is), the larger your batch size, the larger your learning rate can be without negatively affecting performance.
For the rest of the experiments, thanks a lot, the comparison is very clear and this will be very helpful for those of us who want to have some "default" parameters to start from. I see a clear advantage to leave the learning rate as None, as when setting an external learning rate we typically have to make experiments to find the optimal one for that concrete problem, so if it provides results similar or better to the ones provided by the paper's recommended parameters, I'd go with that as default.
I hope I can have my experiments done soon, which will be with t5-large probably, to see if they coincide with your findings.

[stas00]

OK, let's merge this and if we need to make updates for any new findings we will do it then.

[jsrozner]

Although I didn't really run an experiment, I have found that my settings for adafactor (relative step, warmup, scale all true) do well when training t5-large, also.

@alexvaca0 please post your results when you have them!