Repository navigation
[WIP] Add BertIterator (MultiDataSetIterator for BERT training) - #7430
Conversation
treo
left a comment
treo
left a comment
There was a problem hiding this comment.
Overall a good start. I'm mostly nitpicking on documentation and some edge case checks.
Concerning the tasks I'm not quite satisfied.
From my understanding of the paper, the big thing about Bert is that they pre-train using both the MLM / Cloze task and next sentence classification task at the same time. This can also be seen when they create training examples in https://github.com/google-research/bert/blob/master/create_pretraining_data.py#L219. They are using the same masked input for both tasks there.
The SEQ_CLASSIFICATION task also doesn't look like it is not an instance of the next-sentence classification task.
To make this a proper BertIterator in my opinion we need to add both the next-sentence classification task, as well as allow multi task training.
| * <b>RANK2_IDX</b>: return int32 [minibatch, numTokens] array with entries being class numbers<br> | ||
| * <b>RANK3_NCL</b>: return float32 [minibatch, numClasses, numTokens] array with 1-hot entries along dimension 1<br> | ||
| * <b>RANK3_NLC</b>: return float32 [minibatch, numTokens, numClasses] array with 1-hot entries along dimension 2<br> | ||
| * <br> |
There was a problem hiding this comment.
Maybe add a short explanation when someone would use each of those options
|
@treo Thanks for the review
Yes, that is my understanding also, and the omission was intentional at this point. I want to get something usable sooner rather than later, even if it's not optimal. My main goal for now was the sequence classification task as that's what we'll be using initially for transfer learning. As for supporting the "next sentence classification" task, I want that. But it'll be more complex to implement: instead of iterating over sentences, we'll want to iterate over documents, and we'l need some automated way to break text into sentences. So, future additions we'll need here:
There's some non-trivial challenges with those, and I planned to address them separately. (Javadoc should reflect that, however)
It's not. It's designed for transfer learning, single sequence in, single label out... I'll clarify that. |
1d06db3 to
dc6d529
Compare
|
Great, with the clarification that the intent wasn't to support all the unsupervised pre-training options yet, and the fixes in the java doc, only the question of when you'd use RANK3_NLC, my guess is that some of the imported models may need it? |
2 participants
Fixes: #7287