Skip to content

feat: support embedding interface for all generate LLM models.#990

Merged
XuZhang99 merged 2 commits intojd-opensource:mainfrom
RobbieLeung:feat/embedding
Mar 10, 2026
Merged

feat: support embedding interface for all generate LLM models.#990
XuZhang99 merged 2 commits intojd-opensource:mainfrom
RobbieLeung:feat/embedding

Conversation

@RobbieLeung
Copy link
Copy Markdown
Collaborator

TODO: support common embedding's paramters like vllm/sgalng.

Copy link
Copy Markdown
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 refactors the model hierarchy by removing the separate EmbeddingLM class and integrating its functionality into the CausalLM base class. This is a good architectural improvement that unifies the model interfaces and reduces code duplication. The changes are mostly correct, but I found a critical compilation error in one of the files.

Comment thread xllm/models/llm/qwen3.h Outdated
@yingxudeng yingxudeng self-requested a review March 4, 2026 02:41
yingxudeng
yingxudeng previously approved these changes Mar 4, 2026
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Copy link
Copy Markdown
Collaborator

@yq33victor yq33victor left a comment

Choose a reason for hiding this comment

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

LGTM

@XuZhang99 XuZhang99 merged commit 7e2c710 into jd-opensource:main Mar 10, 2026
20 of 27 checks passed
@RobbieLeung RobbieLeung deleted the feat/embedding branch March 11, 2026 12:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants