Skip to content

ci(review): strengthen model review invariants - #2994

Merged
akoumpa merged 2 commits into
mainfrom
akoumpa/ci/review-random-init-invariants
Jul 10, 2026
Merged

ci(review): strengthen model review invariants#2994
akoumpa merged 2 commits into
mainfrom
akoumpa/ci/review-random-init-invariants

Conversation

@akoumpa

@akoumpa akoumpa commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

Summary

  • require checkpoint-free model paths to initialize every active parameter, persistent buffer, and routing table before first use
  • require model-specific logic to live under nemo_automodel/components/models/<model>/ and classify violations elsewhere as critical findings
  • standardize tensor-layout documentation on forms such as hidden_states: Tensor of shape [batch, sequence, hidden]., allow conventional dimension names without definitions, and require a compliant rewrite example in review findings
  • require focused no-checkpoint state coverage and a finite first forward/backward on a representative backend or topology

Why

PR #2991 exposed a gap in checkpoint-free initialization review: DeepSeek-V4 random-init training left HyperConnection tensors uninitialized and left the HashGate route table all zeros, which violated DeepEP routing invariants and produced a NaN gradient norm on the first backward pass.

The review prompt also lacked a strict ownership gate for model-specific behavior and prescribed symbolic tensor layouts without a single reusable documentation format. These additions make model boundaries and tensor contracts concrete and actionable.

Developer impact

Model-specific architecture, initialization, weight-layout, and parallelization policy must stay with the owning model package. Tensor docstrings use a consistent semantic shape form, while review findings for unclear layouts must show an acceptable rewrite. Model changes that add parameters, buffers, routing tables, or random-init paths must demonstrate valid checkpoint-free initialization.

Validation

  • git diff --check -- .github/workflows/claude-review.yml
  • parsed .github/workflows/claude-review.yml with yaml.safe_load
  • asserted that the rendered prompt contains the model ownership path, critical severity, common tensor-layout example, and required review rewrite example

Follow-up to #2991 and NVBug 6317402.

Signed-off-by: Alexandros Koumparoulis <akoumparouli@nvidia.com>
@copy-pr-bot

copy-pr-bot Bot commented Jul 9, 2026

Copy link
Copy Markdown

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

@akoumpa

akoumpa commented Jul 9, 2026

Copy link
Copy Markdown
Contributor Author

Blind review-prompt validation

I ran the updated .github/workflows/claude-review.yml prompt in a fresh subagent without providing the known bug root cause or fix. The subagent reviewed the complete 26-file, non-uv.lock DeepSeek-V4 onboarding diff (17ed5796^..17ed5796, PR #2039) and inspected the corresponding code on current origin/main (1197b628).

The prompt produced two High findings:

  1. Checkpoint-free initialization leaves backbone and MTP HyperConnection parameters uninitialized.

    • DeepseekV4HyperConnection and DeepseekV4HyperHead allocate parameters with torch.empty.

    • The production initialization path does not initialize those tensors in the backbone, final head, or MTP blocks.

    • A small CPU probe that constructed the model without checkpoint loading and invoked the production initializer reported:

      attn_hc.fn finite=False
      logits finite=False
      loss=nan
      gradients finite=False
      
  2. Checkpoint-free HashGate routing initializes every top-k slot to expert 0.

    • tid2eid is created as zeros and reset to zeros by init_weights().

    • With topk=2, the production initializer probe returned:

      [[0, 0], [0, 0], [0, 0]]
      
    • This violates the downstream distinct-experts-per-token routing requirement used by DeepEP. The existing test checked only the route dtype, not range or uniqueness.

Result: the new checkpoint-free initialization guideline flags both root causes fixed by #2991: active torch.empty state that is never initialized and an all-zero routing placeholder that violates the dispatcher's structural contract.

This was a faithful local subagent simulation of the review prompt, not a hosted GitHub Actions invocation.

@akoumpa akoumpa added the docs-only With great power comes great responsibility. label Jul 9, 2026
@akoumpa

akoumpa commented Jul 9, 2026

Copy link
Copy Markdown
Contributor Author

/ok to test dd79412

Signed-off-by: Alexandros Koumparoulis <akoumparouli@nvidia.com>
@akoumpa akoumpa changed the title ci(review): require random-init invariant checks ci(review): strengthen model review invariants Jul 10, 2026
@akoumpa
akoumpa marked this pull request as ready for review July 10, 2026 11:05
@akoumpa
akoumpa requested a review from a team as a code owner July 10, 2026 11:05
@akoumpa

akoumpa commented Jul 10, 2026

Copy link
Copy Markdown
Contributor Author

/ok to test 88899b6

@akoumpa
akoumpa merged commit 14eb6dc into main Jul 10, 2026
38 checks passed
@akoumpa
akoumpa deleted the akoumpa/ci/review-random-init-invariants branch July 10, 2026 11:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

docs-only With great power comes great responsibility.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants