add emb_dim in modelConfig

[yinjuncheng]

No description provided.

[LLLLKKKK]

Code Review: PR #811 — add emb_dim in modelConfig

Reviewer: Automated Code Review (Claude)

---

Summary

This PR adds "emb_dim" to ModelConfig._python_fields, allowing model_config.emb_dim to be set without triggering the AttributeError guard in __setattr__. The change is a single-line addition in rtp_llm/config/model_config.py.

---

Findings

1. Missing default initialization in __init__ (Medium)

Every other field in _python_fields is initialized with a default value in ModelConfig.__init__() (e.g., self.inter_size: int = 0, self.model_name: str = ""). emb_dim is not initialized, which means reading model_config.emb_dim before it is explicitly set will raise AttributeError. This is inconsistent with the existing pattern.

Suggested fix — add to __init__:

self.emb_dim: int = 0  # or another appropriate default

2. No current consumer on ModelConfig (Low)

The only existing usage of emb_dim in the codebase is in BiEncoderTbstars.__init__ (internal_source/rtp_llm/models/flot.py:226), where it reads from config_json and stores it as self.emb_dim on the model instance — not on ModelConfig. It would be helpful to clarify in the PR description which code path will set/read model_config.emb_dim.

3. No tests added (Low)

No tests were added or modified. Consider adding a simple test verifying that ModelConfig can get/set emb_dim with the expected default.

---

Verdict

Needs minor revision. The change is safe and backward-compatible, but emb_dim should be initialized in __init__ with a default value to match the pattern of all other _python_fields and avoid potential AttributeError on read.

[LLLLKKKK]

🤖 AI Code Review — PR #811 add emb_dim in modelConfig

概述

在 ModelConfig 中注册 emb_dim 为 Python-only 字段，并在 __init__ 中初始化为 0。改动极小（1 文件，+2 行），目的是允许在 ModelConfig 实例上设置 emb_dim 属性。

建议改进

P1: 默认值不一致

ModelConfig.__init__ 中 emb_dim 默认值为 0，但现有 BiEncoderTbstars.__init__（internal_source/rtp_llm/models/flot.py:226）中从 config_json 读取时默认值为 256：

# ModelConfig (本 PR)
self.emb_dim: int = 0

# BiEncoderTbstars (现有代码)
self.emb_dim = config_json.get("emb_dim", 256)

如果后续代码迁移为从 config.emb_dim 读取，默认值 0 可能导致 embedding 截断维度为 0，产生静默错误。建议将默认值改为 256 以与现有行为保持一致，或在使用处添加 emb_dim == 0 的校验。

P1: 缺少消费者代码

当前 codebase 中没有任何代码读取 config.emb_dim（ModelConfig 实例的属性）。唯一使用 emb_dim 的地方是 BiEncoderTbstars 中设置在模型实例上的 self.emb_dim。

分支名 hotfix/fix_biencoders 暗示这是一个 bugfix，但仅添加字段定义而没有消费侧改动，无法判断是否真正修复了问题。如果是为后续 PR 做准备，建议在 PR 描述中说明。

P1: 缺少 PR 描述和测试

• PR body 为空，没有说明修改动机和关联的 bug。
• 没有添加测试来验证 emb_dim 字段的行为。

总结

改动本身不会引入 regression，但默认值与现有代码不一致（0 vs 256）是潜在风险点。建议补充 PR 描述说明使用场景，确认默认值选择，并考虑同时提交消费侧代码。

[LLLLKKKK]

🤖 AI Code Review — PR #811
Head SHA: c79acc9a7439 | Verdict: LGTM

Summary

Adds emb_dim field to ModelConfig as a Python-only field with default 0. Follows existing patterns correctly. No issues found.