[YouHengfei]

Purpose

Restore Nanbeige PD startup after #118 by creating a unified ModelConfig before worker startup and passing it through deployment initialization.

The regression reconstructed every worker's configuration as OuroConfig; a Nanbeige payload fails with TypeError: unexpected keyword argument 'num_loops'. Nanbeige PD was supported before the refactor through #77.

Required human contributor answers

1. What outcome are you trying to achieve?

Restore PD serving with Nanbeige checkpoints while preserving model-neutral configuration ownership.

2. What is the highest-leverage variable for that outcome?

Create and validate model configuration at the deployment entry point, so request validation and worker initialization consume the same configuration.

3. How will this work move that variable rather than create busywork?

Implement the requested initialization contract: retain model-specific configuration inside a unified ModelConfig, pass it to the coordinator and workers, and reserve worker ready messages for runtime results.

4. Who is the customer or user, and what do they actually need?

Nanbeige PD users need their checkpoints to start and generate correctly, as supported before the refactor.

PR necessity and bugfix evidence

This fixes a startup regression. The contributor reproduced the failing configuration conversion at the CPU metadata boundary:

OuroConfig(**asdict(NanbeigeConfig()))
# TypeError: unexpected keyword argument 'num_loops'

Implementation

Deployment entry point → create and validate ModelConfig
    ├─ StageClient / PDEngine → request validation
    └─ worker → load weights and initialize execution
Worker ready → runtime KV capacity, transport metadata and fingerprints

• ModelConfig retains the concrete Ouro, Nanbeige or Huginn configuration. Common limits delegate to its fields and derived properties.
• Python and serving entry points prepare configuration before spawning workers. Workers reuse the supplied concrete configuration when loading checkpoint weights; module inputs preserve their supplied weights.
• Coordinators no longer reconstruct model configuration from ready messages. P/D worker-computed weight/KV fingerprint checks remain in place.
• Unsupported Huginn PD deployments fail before workers are spawned.
• Regression tests cover configuration propagation across spawn, derived properties, exact checkpoint weights, ready messages without model configuration, and Nanbeige request validation. docs/unified_execution.md documents the ownership contract.

Validation

Reviewed commit: af665110067fc47776207593d78d107a1bbe17a9
PR baseline: 4a350344027960a48384626aa646f1765fda6d4f

Contributor-reported results on the updated implementation

Validation report:

• CPU suite: 834 passed, 329 skipped.
• PD GPU suite on two H100s: 38 passed, 1 skipped; the skipped case requires four GPUs.
• Fresh GSM8K-100 native/PD comparisons: Ouro-1.4B 69/100 and Nanbeige4.2-3B 63/100. For each model, native and PD token and exit-depth sequences matched on all 100 questions.

These results establish agreement for the tested configurations, not general equivalence across all workloads.

Independent reviewer verification

On the reviewed commit, Python 3.12.3, Torch 2.13.0+cu130 and Transformers 4.55.0:

python -m pytest -q tests/test_auto_model.py tests/test_stage_engine.py tests/test_pd.py -m 'not gpu'

96 passed, 34 deselected in 18.89s. GPU tests and GSM8K were not rerun by the reviewer.

The commit's lint and CPU CI check also passed.

[bjf-frz]

Please create a unified ModelConfig at the entry point and pass it through to the coordinator and workers.

The current flow is:

Entry point → start workers → worker creates model config
→ serialize config into the ready message
→ coordinator reconstructs PDModelInfo

The expected flow is:

Entry point → create and validate ModelConfig
    ├─ single-stage engine / PD coordinator: validate requests
    └─ workers: load the model and initialize execution

The model configuration is already knowable before worker startup. The coordinator should not need a worker round trip to recover it, or introduce a separate PDModelInfo representation of the same configuration.

Please keep model-specific configuration within the unified ModelConfig and pass that object through the initialization chain. The worker’s ready response should report runtime results, such as allocated KV capacity and transport metadata.

[0z5a]

Following up on @bjf-frz's comment above: I agree with the unified ModelConfig direction. Please treat this as a continuation of that review rather than as an approval of the current approach. I withdraw my earlier suggestion to send the limits from the worker, because it would add to the worker round trip that comment asks to remove.

Some pointers that may help with the change:

• The entry points already resolve the config and drop it.
  • PDEngine.__init__ calls model, revision, _ = resolve_model_config(model, revision=revision) (vllm_rlt/pd/engine.py), and entrypoints/llm.py does the same. The third value is the parsed config.json, discarded with _.
  • entrypoints/serve.py and entrypoints/pd_serve.py already keep it as config.
  • Every family has Config.from_dict (ouro.py, nanbeige.py, huginn.py).
  • So the entry point can build the family config once: from a path via from_dict, use a config dataclass as is, or take module.config from an nn.Module. It can then pass that object to the coordinator / single-stage engine and to the workers. Today the worker resolves and parses config.json a second time, through AutoModelForCausalLM.from_pretrained → resolve_model_config.
• With that object, PDModelInfo and from_ready are no longer needed. add_request() can validate against the ModelConfig directly. Attribute access also covers families that expose limits as properties, such as Huginn's total_ut_steps and max_position_embeddings, which asdict drops from the current ready payload.
  • Whether PD supports Huginn (#69) then becomes an explicit model-type check, not a side effect of the metadata shape.
  • The "model metadata lacks" expectation in the new test would change accordingly.
• The ready message can then carry only runtime results. That means KV blocks / block size / depths, transport info, and the P/D compatibility fingerprint computed on the worker. Its model=asdict(...) field can go.
• The single-stage path has the same dependency. StageClient rebuilds model_config as SimpleNamespace(**message["model"], ...) from the ready message (vllm_rlt/engine/llm_engine.py), and entrypoints/cli.py reads llm.engine.model_config.total_ut_steps from it. Passing the entry ModelConfig through covers this consumer too.

What I verified on the current head, in case it is useful for the rework:

• The regression comes from c3ce155 (#118), which replaced the coordinator-side three-field model info with OuroConfig(**ready["model"]).
• PDEngine.model_config is only read in add_request().
• The branch merges cleanly with current main.
• tests/test_pd.py passes on CPU: 14 passed, 25 skipped.
• The Nanbeige startup test, which mocks only the process boundary, is a good shape to keep. It can assert that request validation uses the entry ModelConfig and that startup no longer needs model in the ready message.

[0z5a]

Requesting changes to replace my earlier approval, which predates the direction set in @bjf-frz's comment above. Please rework this around a unified ModelConfig created at the entry point and passed to the coordinator and the workers, as described there. The worker ready message should then carry only runtime results. My review above lists where the config is already resolved at the entry points and which consumers depend on the ready-message model payload.

[YouHengfei]

Thanks @bjf-frz and @0z5a. Reworked in af66511 following the entry-owned configuration flow:

• Entry points create and validate a unified ModelConfig, retaining the concrete model-specific configuration.
• The coordinator and StageClient retain it and pass it to workers. Checkpoint loaders reuse the supplied configuration without reparsing JSON.
• Removed PDModelInfo and the ready-message SimpleNamespace reconstruction. Ready now reports runtime capacity and transport information; worker-computed fingerprint checks remain unchanged.
• Unsupported PD model families are explicitly rejected before spawning workers.

Validation on the updated implementation:

• CPU: 834 passed, 329 skipped.
• PD GPU: 38 passed, 1 skipped (requires four GPUs).
• Fresh GSM8K-100 native/PD comparisons: Ouro 69/100 and Nanbeige 63/100, with identical token and exit-depth sequences on all 100 questions for each model.
• Regression tests cover configuration propagation, derived properties, exact checkpoint weights, and startup without model metadata in ready.

The architecture documentation now reflects this ownership contract. Please take another look.

[0z5a]

Thanks for the rework. af66511 follows the entry-owned flow from @bjf-frz's comment:

• The entry point builds ModelConfig once.
• The coordinator and StageClient keep it and pass it to the workers.
• The checkpoint loaders reuse it without reading config.json again.
• PDModelInfo is gone.
• The ready message now carries only runtime capacity, transport info and the fingerprint.

Huginn's derived limits now come through the properties, and the CLI's model_config.total_ut_steps reads from the same object.

I merged the branch with current main (66920da) and checked it: the full CPU suite passes (835 passed, 369 skipped), and ruff check and format are clean.

Two optional nits, not blocking:

• prepare_model checks that a module or config source matches model_config, but for a path source it trusts the caller. That is fine for the current entry points, which build both from the same resolved checkpoint. A one-line comment would make the contract explicit.
• ModelConfig.model_type builds the full to_dict() on every access. It is only read at startup, so this is cosmetic.

LGTM.

[bjf-frz]

lgtm