Skip to content

[WIP] ci: compare pipeline-parallel Eagle3 convergence - #180

Draft
yubofredwang wants to merge 1 commit into
mainfrom
codex/pp-hidden-state-overlap
Draft

[WIP] ci: compare pipeline-parallel Eagle3 convergence#180
yubofredwang wants to merge 1 commit into
mainfrom
codex/pp-hidden-state-overlap

Conversation

@yubofredwang

Copy link
Copy Markdown
Collaborator

Summary

  • Add a four-GPU Eagle3 training consistency gate for pipeline parallelism.
  • Run the same deterministic workload with tensor parallelism and pipeline parallelism.
  • Compare every optimizer-step loss and fail when the relative difference exceeds 10%.
  • Publish per-step trajectories and convergence diagnostics as CI artifacts.
  • Run the gate in both pull-request GPU integration and nightly coverage.

Validation

  • Python compilation, shell syntax, workflow YAML parsing, Ruff, and comparator unit tests pass locally.
  • GPU execution is delegated to the configured CI runner.

Scope

This change contains no environment-specific paths, hostnames, job identifiers, benchmark claims, or credentials.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 32e834e388

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines +375 to +377
(f"{mooncake_key}_hs", all_hidden[:, :split_at]),
(f"{mooncake_key}_ids", input_ids),
(f"{mooncake_key}_lhs", all_hidden[:, -self._hidden_size :]),

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Normalize hidden states before raw publication

When vLLM is configured with dtype=float16 (supported through inference.vllm.extra_args), these cache slices remain FP16, but put_raw_tensors() performs no conversion while the connector metadata still declares every hidden-state tensor as BF16. Consumers therefore reinterpret the FP16 bytes as BF16 and silently train on corrupted values. The previous EagleMooncakeStore.put() path explicitly converted hidden and last-hidden states to HIDDEN_STATES_STORAGE_DTYPE; preserve that normalization for both the TP tuples here and the PP fragments below before publishing them.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Fixed by normalizing hidden-state tensors to the declared storage dtype before raw publication, with regression coverage for both tensor- and pipeline-parallel layouts.

@yubofredwang yubofredwang changed the title ci: compare pipeline-parallel Eagle3 convergence [WIP] ci: compare pipeline-parallel Eagle3 convergence Aug 26, 2026
@yubofredwang
yubofredwang marked this pull request as draft August 26, 2026 10:15
@torchspec-bot
torchspec-bot force-pushed the codex/pp-hidden-state-overlap branch 2 times, most recently from c570340 to c0e202a Compare August 26, 2026 10:36
Signed-off-by: torchspec-bot <262938024+torchspec-bot@users.noreply.github.com>
@torchspec-bot
torchspec-bot force-pushed the codex/pp-hidden-state-overlap branch from c0e202a to b905adf Compare August 26, 2026 10:51
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.

2 participants