run_lm_finetuning generates short training cases
Author: Rocketknight1
Pull request: no

In the original Tensorflow BERT repo, training cases for the Next Sentence task are generated by [concatenating multiple sentences](https://github.com/google-research/bert/blob/master/create_pretraining_data.py#L219) up to the maximum sequence length. In other words the "sentences" used are actually longer chunks of text split at sentence boundaries, which may include more than one sentence. 

The LM finetuning example script doesn't do this, and just uses two single sentences as a training example, which means that most training examples are significantly shorter than max_seq_length. Would you like me to submit a patch to bring our implementation in line with theirs, or was leaving it out intentional?

Comments:
Sorry, I just realized this was mentioned in the [original PR](https://github.com/huggingface/pytorch-pretrained-BERT/pull/124).

Indeed. Happy to welcome a PR if you want to improve this example!

Working on it now! One question, though: It seems likely that I'll have to make significant changes. The reason for this is there is a significant random element in the concatenation of sentences, and so the exact number of training examples is difficult to know in advance. The concept of an index into the list of training examples also stops making sense because of this. This makes a simple implementation based on a Dataset object difficult.

I can see two possible approaches to resolve this:
1) Before training begins, the script does a pass over the dataset and pregenerates all training cases as InputExample objects. These could also be regenerated each epoch to increase diversity. The training cases could be packed into a Dataset object and sampled with RandomSampler, so we could still have meaningful progress bars. This is similar to Google's original implementation, where the data is pregenerated and stored in example files.

2) The random sampling could occur on the fly at train time. This would avoid the time and storage needed for pregenerating training cases, but would become harder to measure the 'length' of an epoch in advance. 

I think either of these could be different enough that it might be better to implement it as a separate script (though it would still use a lot of the helper functions from the existing script). What do you think?

Hi @Rocketknight1, yes both solutions make sense and it seems better to have independent scripts indeed.
Maybe you can start by drafting an independent script focusing on the pregenerated case and then see if the current `run_lm_finetuning` script can be updated for the on-the-fly case?

I think starting with pregenerated makes sense. However, maybe instead of replacing the old script, maybe we can keep that one as is? The original authors mentioned that concatenating sentences into training examples didn't make sense for their use-case. Possibly their dataset was something like chatbot conversations instead of long documents?

Either way, they (and probably others) have use for the simple "one sentence per example" training system so I don't want to delete it entirely!

Sounds good to me!

@thomwolf I created PR #392 which includes the new functionality.