Pass in position_ids into attention for GPT2#42842
Pass in position_ids into attention for GPT2#42842jackzhxng wants to merge 1 commit intohuggingface:mainfrom
Conversation
|
[For maintainers] Suggested jobs to run (before merge) run-slow: gpt2 |
vasqu
left a comment
There was a problem hiding this comment.
We have a test for this but it is skipped with GPT2 currently, i.e. check if paddingfree is the same as padding which relies exactly on this. I have a PR for this over here #42848
The logic is sound tho, depending on the other PR, it might make more sense to add an explicit test here
|
Sounds good, lmk if you want the test here / feel free to merge |
|
Could you add a small test here directly? My other PR is going a bit beyond what I had originally planned |
ArthurZucker
left a comment
There was a problem hiding this comment.
Indeed let's add a test!
|
The docs for this PR live here. All of your documentation changes will be reflected on that endpoint. The docs are available until 30 days after the last update. |
|
#42848 is now merged and includes the passing of the pos ids (along testing) so I'm closing this one - feel free to reopen or tag me if it does not work as expected |
What does this PR do?
Pass in
position_idsso that the custom attention interface implementations have access to it.Who can review?
@ArthurZucker @Cyrilvallez