feat: Support sending additional outputs from vLLM inference #70

kthui · 2024-11-02T00:17:33Z

What does the PR do?

Add support for sending additional outputs from vLLM. At this step, the following 3 outputs are added:

finish reason
cumulative log probabilities
number of token ids

Checklist

Commit Type:

Check the conventional commit type
box here and add the label to the github PR.

Related PRs:

N/A

Where should the reviewer start?

N/A

Test plan:

A new test is added with this PR for all combinations on setting the 3 additional outputs and verifying the outputs are valid for each combination.

CI Pipeline ID: 20077450

Caveats:

N/A

Background

Outputs supported by vLLM in addition to text output: https://github.com/vllm-project/vllm/blob/v0.6.3.post1/vllm/outputs.py#L14-L40

Related Issues: (use one of the action keywords Closes / Fixes / Resolves / Relates to)

N/A

* [WIP] Add additional outputs to auto complete * [WIP] Use individual input tensor to control per additional output * [WIP] Parse additional output flags from request

* Add additional outputs test * Update copyright * Some test enhancement and notes

ci/L0_additional_outputs_vllm/additional_outputs_test.py

ci/L0_additional_outputs_vllm/test.sh

rmccorm4 · 2024-11-06T23:58:15Z

docs/additional_outputs.md

+# OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+-->
+
+# Additional Outputs from vLLM


Excellent documentation and concise reference from top-level README!

rmccorm4 · 2024-11-07T00:08:34Z

ci/L0_additional_outputs_vllm/additional_outputs_test.py

+            # TODO: vLLM may return token ids identical to the previous one when
+            #       streaming, for example:
+            #
+            #       prev: None
+            #       curr: text=' the', token_ids=array('l', [5])
+            #
+            #       prev: text=' the', token_ids=array('l', [5, 1385])
+            #       curr: text=' the term', token_ids=array('l', [5, 1385])
+            #
+            #       prev: text=' the term', token_ids=array('l', [5, 1385, 44])
+            #       curr: text=' the term', token_ids=array('l', [5, 1385, 44])
+            #
+            #       prev: text=' the term', token_ids=array('l', [5, 1385, 44, 48])
+            #       curr: text=' the term “', token_ids=array('l', [5, 1385, 44, 48])
+            #
+            #       If this is no longer the case in a future release, change the assert
+            #       to assert num_token_ids > 0.


I'm not totally following this section. Can you show me an example output from calling /generate_stream on a vllm model with output_num_token_ids=True and stream=True?

On 0.5.3.post1:

text: ' the' num_token_ids: 1 ----- text: ' term' num_token_ids: 1 ----- text: '' num_token_ids: 1 ----- text: ' “' num_token_ids: 1 ----- text: 'p' num_token_ids: 1 ----- text: 'olar' num_token_ids: 1 ----- text: 'ized' num_token_ids: 1 ----- text: '' num_token_ids: 1 ----- text: '”' num_token_ids: 1 ----- text: ' is' num_token_ids: 1 ----- text: ' used' num_token_ids: 1 ----- text: ' to' num_token_ids: 1 ----- text: ' refer' num_token_ids: 1 ----- text: ' to' num_token_ids: 1 ----- text: ' a' num_token_ids: 1 ----- text: ' polar' num_token_ids: 1 -----

which is expected, but on 0.5.5:

text: ' the' num_token_ids: 1 ----- text: ' term' num_token_ids: 0 ----- text: '' num_token_ids: 0 ----- text: ' “' num_token_ids: 0 ----- text: 'p' num_token_ids: 0 ----- text: 'olar' num_token_ids: 0 ----- text: 'ized' num_token_ids: 0 ----- text: '' num_token_ids: 0 ----- text: '”' num_token_ids: 0 ----- text: ' is' num_token_ids: 0 ----- text: ' used' num_token_ids: 0 ----- text: ' to' num_token_ids: 0 ----- text: ' refer' num_token_ids: 0 ----- text: ' to' num_token_ids: 0 ----- text: ' a' num_token_ids: 0 ----- text: ' polar' num_token_ids: 0

It appears the previous output (the token_ids field) is overwritten by the current output, when the engine is streaming outputs for a request.

The issue appears fixed on a later release, i.e. 0.6.3.post1:

text: ' the' num_token_ids: 1 ----- text: ' term' num_token_ids: 1 ----- text: '' num_token_ids: 1 ----- text: ' “' num_token_ids: 1 ----- text: 'p' num_token_ids: 1 ----- text: 'olar' num_token_ids: 1 ----- text: 'ized' num_token_ids: 1 ----- text: '' num_token_ids: 1 ----- text: '”' num_token_ids: 1 ----- text: ' is' num_token_ids: 1 ----- text: ' used' num_token_ids: 1 ----- text: ' to' num_token_ids: 1 ----- text: ' refer' num_token_ids: 1 ----- text: ' to' num_token_ids: 1 ----- text: ' a' num_token_ids: 1 ----- text: ' polar' num_token_ids: 1

I think num_token_ids could probably be better named to reflect that it is the output tokens. OpenAI APIs return information about both input (prompt/context) and output (decode, generation) tokens - so we should leave room for that with clear naming, even if we only implement the output tokens in this PR.

How about, num_output_tokens, and in future if requested, num_input_tokens?

The original ask is for "number of tokens", but I agree that we could return token_ids instead, because from a feature perspective we are making the "return_*" space less crowded if the outputting token_ids would be requested in the future, in addition to num_token_ids.

Sorry edited my responses above. While returning token_ids is probably more generalized, for large sequences it could actually be more costly than returning a single number if all they want is the count (for non-streaming) - and would introduce some postprocessing logic requirement if all they want is the count, so I removed that part for now. I'm a little torn on it.

We can probably just stick to the ask of returning the token counts, and consider either verbose logging (debugging) or further outputs for actual token_ids if requested later?

Return token ids instead of number of token ids

The model.py on the CI pipeline has been replaced with the version on this commit.

Yes, we can just add token_ids if requested later. Will update the name num_token_ids to num_output_tokens.

Rename num_token_ids to num_output_tokens

The model.py on the CI pipeline has been replaced with the version on this commit.

krishung5 · 2024-11-07T00:05:08Z

docs/additional_outputs.md

+[here](https://github.com/vllm-project/vllm/blob/v0.6.3.post1/vllm/outputs.py#L26)
+for more details.
+
+To enable, set `output_finish_reason` input tensor to `True`. The reason will be


FYI - In TRT-LLM backend, the input tensors to enable optional outputs are named with "return_XXX" convention. I don't think we must be aligned with the naming though, I'd prefer to use what vLLM users are used to have.

This is our first attempt to send optional outputs on vLLM, we can name the input tensor switches to "return_*". @rmccorm4 what do you think?

Rename output_* to return_*

The model.py on the CI pipeline has been replaced with the version on this commit.

I'm in favor of being as similar across both backends as possible - unless there's a clear framework-specific idiom like Kris mentioned (ex: something vllm users are already comfortable with).

This reverts commit 457eeaa.

kthui added 3 commits October 31, 2024 19:03

Add additional outputs and their input switches to auto complete

10a5b94

* [WIP] Add additional outputs to auto complete * [WIP] Use individual input tensor to control per additional output * [WIP] Parse additional output flags from request

chore: Refactor generate function

892f0d0

Add additional outputs to response

58ee481

kthui force-pushed the jacky-vllm-additional-outputs branch from 264d387 to 9fc7d0b Compare November 2, 2024 00:21

kthui self-assigned this Nov 2, 2024

Add test for additional outputs

5e605ca

* Add additional outputs test * Update copyright * Some test enhancement and notes

kthui force-pushed the jacky-vllm-additional-outputs branch from 9fc7d0b to 5e605ca Compare November 2, 2024 04:04

Add docs for additonal outputs

f35e9c4

kthui added the PR: feat A new feature label Nov 4, 2024

kthui changed the title ~~Support sending additional outputs from vLLM inference~~ feat: Support sending additional outputs from vLLM inference Nov 4, 2024

kthui marked this pull request as ready for review November 4, 2024 22:53

chore: Unify vLLM test names

e6e6404

kthui requested review from rmccorm4, GuanLuo, krishung5 and oandreeva-nv November 5, 2024 02:02

rmccorm4 reviewed Nov 6, 2024

View reviewed changes

ci/L0_additional_outputs_vllm/additional_outputs_test.py Outdated Show resolved Hide resolved

Switch to pytest

44edd6e

rmccorm4 reviewed Nov 6, 2024

View reviewed changes

ci/L0_additional_outputs_vllm/test.sh Outdated Show resolved Hide resolved

pytest to dump additional outputs

1773dea

rmccorm4 reviewed Nov 6, 2024

View reviewed changes

rmccorm4 reviewed Nov 7, 2024

View reviewed changes

krishung5 reviewed Nov 7, 2024

View reviewed changes

kthui added 4 commits November 6, 2024 16:48

Rename output_* to return_*

29099df

Return token ids instead of number of token ids

457eeaa

Revert "Return token ids instead of number of token ids"

5e9b09f

This reverts commit 457eeaa.

Rename num_token_ids to num_output_tokens

dae3c13

rmccorm4 approved these changes Nov 7, 2024

View reviewed changes

krishung5 approved these changes Nov 7, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Support sending additional outputs from vLLM inference #70

feat: Support sending additional outputs from vLLM inference #70

kthui commented Nov 2, 2024 •

edited

Loading

rmccorm4 Nov 6, 2024

rmccorm4 Nov 7, 2024

kthui Nov 7, 2024

kthui Nov 7, 2024

kthui Nov 7, 2024

rmccorm4 Nov 7, 2024 •

edited

Loading

kthui Nov 7, 2024

rmccorm4 Nov 7, 2024 •

edited

Loading

kthui Nov 7, 2024

kthui Nov 7, 2024

kthui Nov 7, 2024

krishung5 Nov 7, 2024

kthui Nov 7, 2024

kthui Nov 7, 2024

rmccorm4 Nov 7, 2024 •

edited

Loading

feat: Support sending additional outputs from vLLM inference #70

Are you sure you want to change the base?

feat: Support sending additional outputs from vLLM inference #70

Conversation

kthui commented Nov 2, 2024 • edited Loading

What does the PR do?

Checklist

Commit Type:

Related PRs:

Where should the reviewer start?

Test plan:

Caveats:

Background

Related Issues: (use one of the action keywords Closes / Fixes / Resolves / Relates to)

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rmccorm4 Nov 7, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rmccorm4 Nov 7, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rmccorm4 Nov 7, 2024 • edited Loading

Choose a reason for hiding this comment

kthui commented Nov 2, 2024 •

edited

Loading

rmccorm4 Nov 7, 2024 •

edited

Loading

rmccorm4 Nov 7, 2024 •

edited

Loading

rmccorm4 Nov 7, 2024 •

edited

Loading