Closed joris-sense closed 2 months ago
Check out this pull request on
See visual diffs & provide feedback on Jupyter Notebooks.
Powered by ReviewNB
(I guess an equivalent change would come into train_idefics2.py
, which I guess should be called train_idefics3.py
)
@joris-sense normally with most of the VLMs we pass either "suffix" or "labels" which indicates we're doing training and it automatically adds an EOS token, I think I missed it here. anyhow, the model implementation is incomplete, and we are currently unifying preprocessors for VLMs (just as a heads up) IDK when they'll be wrapped up.
I think I'll merge this PR, it will solve the most of the issue but I'll put a disclaimer saying this model implementation is pinned on early stage and isn't reliable since we already came across a bunch of things.
@joris-sense you haven't tested this right?
Yep, unfortunately have only tested that this works on my own version of the notebook.
When I try inferring with the old version of the Idefics finetuning notebook (here), the generated text doesn't ever stop, and the model outputs repetitions and garbage apparently forever. To me, this seems to be because no EOS tokens are added to the finetuning data, and I changed the code accordingly (with the change, my notebook seems to work for me).
The current version of the finetuning notebook doesn't work on my machine anyway in this form for apparently several reasons (and doesn't contain inference code), so I couldn't check if it will work with this patch. But it seemed to me that a patch similar to this one is necessary.
I am still not sure whether this output format proposed is correct. In the version I propose (formatting+adding endoftext), the string ends with and a newline as well. @merveenoyan is this the intended formatting?
...textextext<end_of_utterance>\n<|end_of_text|>
, i.e. contains