Question answering tlt
State: closed | Pull request: yes

Changed QAModel to be compatible with tlt:
* particularly added additional inference function 

This pull request **introduces 1 alert** when merging a16eecc6c2fa090ee3d06d4ae9063366ecec5402 into fcb5fcec71d7b633fde6cfaaba88857ae79b656e - [view on LGTM.com](https://lgtm.com/projects/g/NVIDIA/NeMo/rev/pr-3cc06276c5c635785dda317471c3755d1528e61d)

**new alerts:**

* 1 for Unused import

> @VahidooX left plenty of good comments.
> 
> My main concern is the "eval" - it would be great if we would agree on meaning/role of this split and stick to it in all our examples.
> 
> I would go with `validation` for the splits that are used for validation during training.

Thanks @VahidooX @tkornuta-nvidia for your review. I addressed the "eval concern".  I created now TRAINING_MODE, EVALUATION_MODE and INFERENCE_MODE. This is not the same as the datasplit. The mode determines what dataset type to create, e.g. TRAINING_MODE is for training data and assumes only 1 label/answer. EVALUATION_MODE is used for both validation and test data and expects 1 or more possible labels. INFERENCE_MODE creates a dataset without labels.

By doing this SQuAD requires 1 Dataset for all modes, which I find cleaner. There is unfortunately no consistency between models. AFAIK text classification dataset switches between modes by checking whether a file (assumes labeled data) or queries (assumes unlabeled data) is passed.

For token classification dataset yet another approach is used, which is to use 2 different classes for labeled and unlabeled data respectively.

> I created now TRAINING_MODE, EVALUATION_MODE and INFERENCE_MODE. This is not the same as the datasplit. The mode determines what dataset type to create, e.g. TRAINING_MODE is for training data and assumes only 1 label/answer. EVALUATION_MODE is used for both validation and test data and expects 1 or more possible labels. INFERENCE_MODE creates a dataset without labels.

Ok, thought about it some more and need to reconcile my answer. Introduction of those modes seems a bit artificial.
Why we cannot got with e.g. "validation and test splits", what are we gaining by putting them into the same "EVALUATION_MODE" bucket?



> By doing this SQuAD requires 1 Dataset for all modes, which I find cleaner. There is unfortunately no consistency between models. AFAIK text classification dataset switches between modes by checking whether a file (assumes labeled data) or queries (assumes unlabeled data) is passed.

I totally agree.

I have once concern though: why `MODE`? what mode means in this context? I honestly think in this case we are talking about different `splits`...

My understanding is:
 - you start with the `dataset` (contains all data)
 - you split dataset into smaller subsets (for training, validation, testing etc.) => hence `split`

Additionally, some datasets might contain `variants`, like e.g. in Visual Reasoning in CLEVR aside of the "main CLEVR variant" you have CoGenT-A and CoGenT-B variants with different combinations of object features... makes sense?

> By doing this SQuAD requires 1 Dataset for all modes, which I find cleaner. There is unfortunately no consistency between models. AFAIK text classification dataset switches between modes by checking whether a file (assumes labeled data) or queries (assumes unlabeled data) is passed.
> 
> For token classification dataset yet another approach is used, which is to use 2 different classes for labeled and unlabeled data respectively.

Ok, maybe this is a good moment to bring it/clean it up a bit? Adding that as an "identified" issue in Iteration 6...