SQuAD convert_examples_to_features skipping doc tokens when they exceed max_seq_length
Author: ofirzaf
Pull request: no
Labels: name: wontfix

## 🐛 Bug

<!-- Important information -->

Model I am using (Bert, XLNet....): BERT

Language I am using the model on (English, Chinese....): English

Using Transformers v2.3.0 installed from pypi

The problem arise when using:
transformers/data/processors/squad.py + BertTokenizer

The tasks I am working on is:
*  SQuADv1.1

## To Reproduce

Steps to reproduce the behavior:

run:
```SQUAD_DIR=$HOME/data/SQUAD
export CUDA_VISIBLE_DEVICES=0

python run_squad.py \
  --model_type bert \
  --model_name_or_path bert-base-uncased \
  --do_eval \
  --do_lower_case \
  --predict_file $SQUAD_DIR/dev-v1.1.json \
  --per_gpu_eval_batch_size 1 \
  --max_seq_length 128 \
  --doc_stride 128 \
  --output_dir $HOME/tmp/debug_squad \
  --overwrite_output_dir
```

## Expected behavior
Looking at the first example:
    Q:"Which NFL team represented the AFC at Super Bowl 50?" 
    Doc: "Super Bowl 50 was an American football...") 
After converting the example to features I see that the question and doc lengths after tokenization are 11 and 157 and in addition each feature need 3 extra tokens for the [CLS], [SEP] tokens. So I would expect the first feature to be: 
`[CLS] [11 Q tokens][SEP][114 Doc tokens][SEP]` = total of 128 tokens
and the second feature to be:
`[CLS][11 Q tokens][SEP][43 Doc tokens][SEP][Padding]` = total of 57 tokens without padding.

Currently the implementation of squad and the tokenizer skips doc_tokens[115:128] as if the first 128 doc tokens appeared in the first features:
the second feature is:
`[CLS][11 Q tokens][SEQ][last 29 Doc tokens][SEP][Padding]` = total of 43 tokens

This bug happens in all the examples, the stride is not done correctly and some of doc tokens are skipped.

## Environment

* OS: 16.04.6 LTS
* Python version: 3.6.8
* PyTorch version: 1.3.1
* PyTorch Transformers version (or branch): 2.3.0
* Using GPU 1
* Distributed or parallel setup no
* Any other relevant information:


Comments:
Indeed, this looks like a bug, thank you for opening an issue. I'll take a look at it.

This issue stems from the two arguments: `max_seq_length=128` and `doc_stride=128`.

Would you mind telling me the expected behavior when putting a doc stride as big as the maximum sequence length? Since the sequence length considers both the document and the question, I don't see a reason for putting such a high document stride: it is larger than the maximum document length that will be in the sequence, and therefore breaks the stride.

According to your use-case we'll do our best to accommodate it with our script.

You are correct, I would expect a warning or an error in this case.
Anyway, even if the `doc_stride` was less than the `max_seq_len` there will still be a possibility that doc tokens will be skipped in the current implementation so I think a warning should occur. I would expect that `doc_tokens` will never be skipped but that's me

Indeed, I agree with you that raising a warning, in this case, would be best. Thanks for your feedback!

I've added a warning in 6e2c28a

I think the solution is to specify the question token max length  and
padding the question part to the max question token length, then you can
just set the max doc_stride is max seq_length - max question length . It
will not loss any information

On Fri, Jan 17, 2020 at 03:00 Lysandre Debut <notifications@github.com>
wrote:

> I've added a warning in 6e2c28a
> <https://github.com/huggingface/transformers/commit/6e2c28a14a3d171e8c4d3838429abb1d69456df5>
>
> —
> You are receiving this because you are subscribed to this thread.
> Reply to this email directly, view it on GitHub
> <https://github.com/huggingface/transformers/issues/2548?email_source=notifications&email_token=AIEAE4FWVAXHHAENIDH2JETQ6CVFNA5CNFSM4KHW5362YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEJFFA4Y#issuecomment-575295603>,
> or unsubscribe
> <https://github.com/notifications/unsubscribe-auth/AIEAE4HZCAYY5EHQTOLRQEDQ6CVFNANCNFSM4KHW536Q>
> .
>


This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.