Add inputs embeds in generation #30269
Conversation
|
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. |
| inputs_embeds = self.embed_tokens(input) | ||
| inputs_embeds = inputs_embeds * self.embed_scale |
There was a problem hiding this comment.
I don't think we change this -- it is a breaking change; the same inputs_embeds input will yield a different output after this PR on a well-established model.
Does it solve any problem from the VLM side?
There was a problem hiding this comment.
No, afaik these models are not used as backbones in VLMs, so prob no real benefit for now.
Close the PR?
There was a problem hiding this comment.
@zucchini-nlp We can keep the PR, it's useful! Not a priority, but since it's done let's add it :D
Let's just revert this line 🤗
There was a problem hiding this comment.
Hmm that will make fail tests for equivalence between input_ids and input_embeds. Then the better decision if to disable totally inputs embeds for these models. Okay, will do
|
(CI failing due to hub timeouts, I consider it to be equivalent to all green) |
| if inputs_embeds is not None and past_key_values is None: | ||
| model_inputs = {"inputs_embeds": inputs_embeds} | ||
| else: | ||
| model_inputs = {"input_ids": input_ids.contiguous()} |
There was a problem hiding this comment.
Interesting - what's the reason for needing to add .contiguous here?
There was a problem hiding this comment.
Not much of a reason right now, as it is part of compile compatibilty. Decided to leave in case these models will be reworked for compile also
|
Comments are addressed and test passing, so merging now |
* Add inputs embeds in generation * always scale embeds * fix-copies * fix failing test * fix copies once more * remove embeds for models with scaling * second try to revert * codestyle
What does this PR do?
Not all decoder-only language models support passing "inputs_embeds" currently. I believe for consistency we can add a possibility. However this PR does not do anything for multimodal LLMs, as those are much more complicated sometimes and there may be no reasons for passing "inputs_embeds" manually.