Skip to content

Conversation

@noooop
Copy link
Collaborator

@noooop noooop commented Dec 15, 2025

Improve all pooling task

These PRs are mostly conflicting with each other, so combining them into a series would better inform reviewers about what happened. And what else needs to be done after that?

Purpose

Generate runner supports using embed and token_embed tasks & End the Improve all pooling tasks series

FIX #11905
FIX #24288
FIX #6165
FIX #4435

Test Plan

tests/models/language/pooling/test_extract_hidden_states.py

Test Result

pass


Essential Elements of an Effective PR Description Checklist
  • The purpose of the PR, such as "Fix some issue (link existing issues this PR will resolve)".
  • The test plan, such as providing test command.
  • The test results, such as pasting the results comparison before and after, or e2e results
  • (Optional) The necessary documentation update, such as updating supported_models.md and examples for a new model.
  • (Optional) Release notes update. If your change is user facing, please update the release notes draft in the Google Doc.

Signed-off-by: wang.yuqi <[email protected]>
@noooop noooop force-pushed the Prompt_Hidden_States branch from 096de98 to 0346166 Compare December 15, 2025 07:20
@noooop noooop changed the title [Model][Last/N] Improve all pooling task | Support Returning Prompt Hidden States [Model][Last/N] Improve all pooling task | Generate runner supports using embed and token_embed tasks. Dec 15, 2025
Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request updates a test for pooling tasks to align with recent improvements, including support for returning prompt hidden states during generation. The test now uses the 'generate' runner and includes a new case to verify text generation with prefix caching.

My main feedback is that the new test case, while intended to verify the return of prompt hidden states, lacks assertions to confirm their presence and correctness. I've suggested adding these assertions to ensure the feature is properly tested.

Signed-off-by: wang.yuqi <[email protected]>
@noooop noooop force-pushed the Prompt_Hidden_States branch from 63038d5 to 64da7a6 Compare December 15, 2025 09:26
@noooop
Copy link
Collaborator Author

noooop commented Dec 15, 2025

@DarkLight1337

Are we planning to implement generate runner support using embed and token_embed tasks?

With this very very dirty fix can make the tests/models/language/pooling/test_extract_hidden_states.py test pass, but we can't batch the generated request and pooling request together.

Signed-off-by: wang.yuqi <[email protected]>
Signed-off-by: wang.yuqi <[email protected]>
@DarkLight1337
Copy link
Member

but we can't batch the generated request and pooling request together.

I think this limitation is ok if we can alternate between generative and pooling batches

@noooop
Copy link
Collaborator Author

noooop commented Dec 15, 2025

but we can't batch the generated request and pooling request together.

I think this limitation is ok if we can alternate between generative and pooling batches

I will further refine this PR if it's decided to implement “Generate runner support using embed and token_embed tasks”

@noooop
Copy link
Collaborator Author

noooop commented Dec 15, 2025

hello @breakices

PTAL: #24288 (comment)

token_embed can be used to extract Prompt Hidden States

Could using "generate runner support with embed and token_embed tasks" as a form of "Returning Prompt Hidden States" help with RLVR?

@noooop
Copy link
Collaborator Author

noooop commented Dec 15, 2025

hello @charlotte12l

A long time has passed since my last comment #24288 (comment), and I'm finally about to implement it.

Could using "generate runner support with embed and token_embed tasks" as a form of "Returning Prompt Hidden States" help with your use case.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

2 participants