Skip to content

feat: add onerec in supported model docs and align rec utility style.#1055

Open
DragonFive wants to merge 2 commits intojd-opensource:mainfrom
DragonFive:docs/rec-model-support-update
Open

feat: add onerec in supported model docs and align rec utility style.#1055
DragonFive wants to merge 2 commits intojd-opensource:mainfrom
DragonFive:docs/rec-model-support-update

Conversation

@DragonFive
Copy link
Copy Markdown
Collaborator

@DragonFive DragonFive commented Mar 13, 2026

Summary

This PR fixes the Rec support documentation and aligns a few Rec-related code paths with the current project style. This PR will merge after #1051 is merged.

Changes

  • remove Qwen2.5 and Qwen3 from the Rec section in:
    • docs/zh/supported_models.md
    • docs/en/supported_models.md
  • keep OneRec as the only documented Rec model entry
  • enable FLAGS_enable_rec_prefill_only = true; in xllm/c_api/internal/rec.cpp
  • replace torch::Dtype with torch::ScalarType in xllm/core/util/utils.cpp
  • replace std::runtime_error with LOG(FATAL) in the same Rec tensor conversion path to match existing code style

Why

  • Qwen2.5 and Qwen3 do not support ILU in the Rec path, so the previous documentation was incorrect
  • the rec.cpp flag should be enabled instead of left commented out
  • utils.cpp should follow the existing repository convention for PyTorch scalar type naming and fatal error handling

Validation

  • checked the Rec section in both Chinese and English support docs
  • verified that xllm/c_api/internal/rec.cpp now enables FLAGS_enable_rec_prefill_only
  • verified that xllm/core/util/utils.cpp no longer uses torch::Dtype or std::runtime_error in the updated path

Notes

  • no build or runtime test was executed in this local macOS editing environment
  • this PR only updates the targeted files and does not address other existing workspace changes

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 updates documentation for supported Rec models, enables a feature flag, and aligns some code with project conventions. The documentation and flag changes appear correct. However, replacing std::runtime_error with LOG(FATAL) in xllm/core/util/utils.cpp introduces a critical issue. While this aligns with some of the file's style, it breaks the error handling in xllm/core/distributed_runtime/rec_master.cpp, which uses a try-catch block to handle these errors gracefully. The new implementation will cause the process to terminate on invalid input, which is a significant regression for a server application. I've added comments with suggestions to revert these specific changes to use throw again.

XuZhang99
XuZhang99 previously approved these changes Mar 13, 2026
zhang-minchao
zhang-minchao previously approved these changes Mar 13, 2026
@DragonFive DragonFive dismissed stale reviews from zhang-minchao and XuZhang99 via a0daea5 March 16, 2026 02:40
@DragonFive DragonFive force-pushed the docs/rec-model-support-update branch from fe21af2 to a0daea5 Compare March 16, 2026 02:40
@DragonFive DragonFive force-pushed the docs/rec-model-support-update branch from a0daea5 to 4d71462 Compare March 26, 2026 01:30
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.

3 participants