T5 compile compatibilty #34089
T5 compile compatibilty #34089
Conversation
There was a problem hiding this comment.
Thank you for this refactor 🙏
I've left a question for us to double-check, but approving since it looks good to me.
main: {all touched models} + {llama, whisper, caches}
| "Liana Barrientos has been married 10 times, nine of them in the Bronx . Her husbands filed for " | ||
| "permanent residence after the marriages, prosecutors say ." |
There was a problem hiding this comment.
Is this a new failure?
If yes: can you confirm a) the generated tokens are different b) compare the logprobs of the different tokens before/after the change, to see whether it can be explained by tiny fluctuations?
If no: can you add a comment with the PR that causes the difference? 🙏
There was a problem hiding this comment.
Added a comment, the regression was caused by changes in the tokenization. So nothing breaks in terms of modeling or generation. Not sure if the fix in tokenization was intended to break T5 (i.e. it actually fixes T5) or we need another PR to fix tokenization back, so I'll leave a comment for future us
|
Will merge this later today if we are all okay. The comment for tokenization is added within code with a TODO for us, in case it is not a bug we can remove the comment later |
* this worked in normal generation, needs more tests * fix almost all tests in t5 * nit * longt5, umt5, mt5 * style * udop, pix2struct * more models * fix some tests * fix onnx tests * tracing tests fixed * compile enabled and tested for t5 models * fix small bug in slow tests * [run-slow] t5 * uncomment * style * update with new generation refactoring * nit * fix copies * this is the fix, had to change t5 to fix copies * update * [run-slow] t5 * [run-slow] t5 * update * add test for encoder only T5 * clean up after rebase * fix pop2piano * add comment * style * fix copies after rebase * fix copies missed this one
What does this PR do?
Same as #33754, I accidentally force pushed to wrong branch and the PR got closed 🙃