EasyNMT icon indicating copy to clipboard operation
EasyNMT copied to clipboard

Update to enable max_length parameter for AutoModels/M2M-100

Open alexwilson1 opened this issue 3 years ago • 3 comments

Bug fix - max_length parameter was not being propagated to AutoModel tokenizers. This was causing OOM for my system on large document inputs.

alexwilson1 avatar May 14 '21 00:05 alexwilson1

Thanks for the PR. There is still I bug I think, that max_length is not passed to model.

I think it might also make sense to pass this as tokenizer arg. Will check in the next days, fix it and merge it.

nreimers avatar May 17 '21 15:05 nreimers

Thanks for getting back to me and thanks for the great library!

I think this fixes the bug for a couple of reasons however:

  1. OpusMT uses this only in the tokenizer arguments also:

inputs = tokenizer(sentences, truncation=True, padding=True, max_length=self.max_length, return_tensors="pt") and this is the only place in the repo the parameter is used.

  1. If the tokenizer is truncating the input, and the tokenizer passes the input to the model, the truncated input is a passed to the model and max_length is correctly applied. In other words, because the tokenizer is the only way to pass data to the model, as long as max_length is causing truncation at the tokenizer, we do not need to pass max_length to the model. My own testing also confirmed that this works.

alexwilson1 avatar May 18 '21 08:05 alexwilson1

Hello there! Any news on this PR ? I'm also facing OOM because the inputs are too long. I would also like to be able to set the maximum length when tokenizing. This PR was in my opinion almost correct. This only missing part is to add the parameter max_length to the method translate_sentence cc @nreimers, @hugoabonizio

nateagr avatar Aug 15 '22 16:08 nateagr