Pass instance metadata through to forward, use it to compute official BiDAF metrics

[matt-gardner]

There are two main things here:

1. I made the metadata that gets associated with each instance available in Model.forward(), if it's requested. I used some reflection in order to make this work without requiring Model.forward() to accept a metadata argument. This is a bit magical, but I didn't really want to require every model to have to accept this argument.

2. I used this metadata inside of BiDAF to compute the official metrics.

[DeNeutoy]

I'm not thrilled with this, but I can see it's necessary for computing Bidaf metrics. There seems to be quite a bit of special casing for the "metadata" key which makes me think that the forward signature is fundamentally wrong, if we need this sort of info to do forward passes of models (or that doing this sort of thing is fine in a separate training script but doesn't belong in the library) and the reflection isn't great as you said. But ultimately I think my only complaint here is more to do with building stuff from params, so LGTM.

[matt-gardner]

We could have a MetadataField, and just remove all of the special casing and reflection, requiring the model to deal with the metadata if it used a reader that produces it... We'll, we'd probably need just a little bit of special casing when producing arrays, but not nearly as much as there is now. That might be a bit cleaner. What do you think?

[DeNeutoy]

One more idea to not have to do this: pass in a character representation of the passage without tokenising it first. This would make the predicted indices line up and remove the need to pass metadata. It's still a bit clunky though, and isn't particularly memory efficient. Is there a reason I haven't thought of why this won't work?

Otherwise, I'm not sure about the MetadataField idea, because ideally you'd want it to be a mixin, as it isn't really something which is constrained to any particular field, and that seems just as obscure as having to special case stuff in what we already have.

[matt-gardner]

I'm not sure what you mean by "a character representation of the passage without tokenizing it first", or how having that would solve the problem.

For the MetadataField, it would be a field that does not get indexed and does not get converted into numpy arrays or pytorch variables, but otherwise holds whatever you want, like a question ID, original passage string, or whatever. Not sure how a mixin would solve anything here.

[DeNeutoy]

The problem is that the predicted char span is for a tokenised version of the passage but the gold char span is for a non-tokenised version of the passage, right? So what if we passed in a non-tokenised character representation of the exact answer as another "label". Maybe I'm not understanding the problem at all, but it seems like it's possible to do exact string matching using indexed tensors where the indexed tensor is a padded version of the exact string answer.

[matt-gardner]

The problem is that I need a predicted character span, and all I have is a predicted token span. I need a way to go from the token span to the character span. I don't know of a way to do that other than keeping track of character offsets.