Adding `logits_to_keep` to older models

[philiproeleveld]

Hi, while working with BioGPT I came across #39016 to not compute logits of the entire sequence when it's not needed. I was wondering if you'd be open to a PR applying the same improvement to BioGPT and, while I'm at it, any other GenerationMixin models that would benefit from this. I understand some of these models might be a bit obsolete and some could benefit from a refactor to modular instead, but I figure while they're there it's worth doing.

[Rocketknight1]

Yes, I think we could support this, and it should be easy enough to do, following the pattern in the GPT PR or newer models that already support it. Feel free to ping me if anyone makes a PR for this!

[VIVEK-MARRI]

Hi, I’d like to work on this issue as part of my first contribution.
My plan is to:

• Add the logits_to_keep argument to the forward methods of the older models that currently don’t support it.
• Update the logits computation to slice accordingly.
• Add/extend unit tests to verify that logits_to_keep works as expected.

Please let me know if there are any specific models or guidelines I should follow before starting.

[Namit24]

yo i wanna take this up

[philiproeleveld]

@VIVEK-MARRI @Namit24 I hope it's not too late to tell you I've already made a start with this actually

[ahmedvictor507]

Hi everyone, I’d like to contribute by implementing logits_to_keep for BioGPT first, following the GPT pattern from #39016. If no one is actively working on it, I can expand to other older models as well. Let me know if that works!

[0x-avi]

hey @Rocketknight1 opened a pr but had a few doubts, firstly i haven't updated the documentation yet, secondly i was only sure that the sequence classification model can allow me to add the slicing logits logic directly, i wanted to know if there was a way more me to test out these changes i make? i tried making a main.py at the root of the repository and doing relative imports and trying to run as a module but some reason the imports with ... failing even though i ran it with the -m flag . if there is another way please let me know so ill make the changes to each of the models in BioGPT and try them out then add another commit to my current pr. Thanks :D

[sonianuj287]

Hi, I’d like to take this up as a first contribution. Can I work on it?

[0x-avi]

uh @Rocketknight1 i think you missed my question above

[Rocketknight1]

Hi @0x-avi sorry, I didn't manually close this issue! It got automatically closed when we merged #41270, because that issue was tagged as fixing this issue.

I'll reopen for now, and if you want to make PRs like the biogpt PR for other older models, feel free. Thank you!