Skip to content

Add DeepSeek-V4 and GLM-5 support - #29

Open
kiddyboots216 wants to merge 1 commit into
codex/oss-fdn-04-moe-minimax-nemotron-20260731from
codex/oss-fdn-05-dsv4-glm5-20260731
Open

Add DeepSeek-V4 and GLM-5 support#29
kiddyboots216 wants to merge 1 commit into
codex/oss-fdn-04-moe-minimax-nemotron-20260731from
codex/oss-fdn-05-dsv4-glm5-20260731

Conversation

@kiddyboots216

@kiddyboots216 kiddyboots216 commented Jul 31, 2026

Copy link
Copy Markdown
Contributor

Summary

Add DeepSeek-V4 and GLM-5 support: sparse-attention kernels and indexers, conversion support, checkpoint handlers, parallelization, registry wiring, and focused model coverage. The shared registry and checkpoint-key hunks are layered on the preceding common-MoE cut.

Validation

  • DeepSeek-V4 and GLM-5 imports passed.
  • Focused suite: 72 passed, 3 declared skips.
  • Review follow-up plain-JSON config-boundary selection: 58 passed.
  • Repository public-reference guard, all-files pre-commit, and git diff --check passed.

Stack

This is 5/9, stacked on the common-MoE/model-family cut.

@kiddyboots216
kiddyboots216 requested a review from qywu July 31, 2026 02:26
@kiddyboots216

Copy link
Copy Markdown
Contributor Author

/broly scan

@broly-code-security-scanner

broly-code-security-scanner Bot commented Jul 31, 2026

Copy link
Copy Markdown

Broly Security Scan

Note

Clean scan
No vulnerabilities detected in this PR.

Note

Re-scan this PR anytime with /broly scan — useful after /broly undismiss, or to refresh findings without a new push.

Broly — SAST (zai-org/GLM-5.2) · Secrets · SCA · IaC · GH Actions · Base Images · Supply Chain Threats · Exploit Chains · Adversarial Verification

We're continuously improving Broly's accuracy and finding quality — your feedback is valuable. False positives, missed findings, bugs, and feature requests all welcome.

Ask in #security-engineering   Powered by Together AI

@qywu qywu left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Reviewed: description is clear, CI passing, no suspicious file changes. LGTM.

@qywu qywu left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Deep review summary

I read through the full ~14.7k-line diff (checkpoint converters/handlers, sparse-attention kernels/indexers for both DSv4 and GLM-5, parallelize/registry wiring, and the new test suites), hand-verified the tensor-shape algebra in the checkpoint fusion/dequantization paths, and cross-checked the shared/common-code edits (deepseek_v3, glm4_moe modeling files) for regression risk.

What holds up: the FP8 block-dequant, MXFP4 unpack, and per-expert gate/up/down fusion transforms in deepseek_v4/checkpoint_handler.py are internally consistent (I traced the shapes by hand — HF [out,in] Linear weights are correctly transposed before stacking into xorl's [E,H,2I]/[E,I,H] layout). The shared-code edits to deepseek_v3/modeling_deepseek_v3.py and glm4_moe/modeling_glm4_moe.py are additive getattr(..., default) reads, so existing V3/GLM4-MoE behavior is preserved. The new end-to-end tests (test_dsv4_hf_to_dcp.py, test_dsv4_loader.py) do real value/shape assertions rather than "just don't crash" smoke checks, and EP divisibility is asserted with clear errors in both the DSv4 and GLM-5 checkpoint handlers and the converter script.

Non-blocking concerns worth a second pair of eyes before this touches production checkpoints:

  1. src/xorl/models/transformers/deepseek_v4/checkpoint_handler.py (_undo_ape_hotfix, ~L727-754) and its test in tests/models/test_dsv4_loader.py (test_ape_hotfix_round_trip, ~L11072-11088). The round-trip test validates _undo_ape_hotfix against _miles_apply_ape_hotfix, a from-scratch reimplementation written in this same PR to mimic what the external "miles" export tool allegedly does. It's internally consistent (I checked the permutation algebra by hand and it does invert), but it never validates against a real HF/SGLang-exported checkpoint or the actual upstream miles source — so if the test author's guess at the forward permutation is wrong, this would silently produce a wrong-but-correctly-shaped compressor.ape for every C4 (DSA) layer, and nothing in the suite would catch it. Given the PR's own validation notes only claim "imports passed" / "72 passed", worth explicitly confirming this against a real Flash/Pro checkpoint (or the miles source) before it's used for real conversions.

  2. src/xorl/ops/dsv4/kernel/tilelang_indexer_fwd.py:7457-7465 (_make_causal_cu_seqlens). The causal boundary for compressed KV groups is cu_seqlen_ke = (positions + 1) // compress_ratio (floor division). This is documented as intentional ("valid compressed groups are [0, (p+1)//compress_ratio)"), and is self-consistent with the rest of the indexer, but I could not find a test that checks this boundary against a known-correct reference for a partially-filled trailing compression group (e.g. verifying group g only becomes visible once its last member position has been produced, not before). An off-by-one here would leak future-token information into the sparse index scores without raising any error — worth a targeted unit test asserting the exact boundary token index if one doesn't already exist upstream in the stack.

  3. Minor: scripts/convert_dsv4_hf_to_dcp.py _maybe_truncate_config (~L84-91) mutates cfg.num_hidden_layers / cfg.compress_ratios directly on an already-constructed DeepseekV4Config, bypassing the constructor's own compress_ratios length/value validation (configuration_deepseek_v4.py ~L1849-1862). Low risk since it's only used for converter smoke/truncation runs, but a truncation that produces an inconsistent length would not be caught the way full-config construction would.

None of the above are proven-wrong logic bugs — they're places where correctness rests on an unverified external contract (the real "miles"/SGLang export format) rather than something checkable from this diff alone. I did not find resource leaks, unsafe deserialization, path traversal, or dead/unwired code — the DSv4/GLM-5 registrations in auto.py and transformers/__init__.py are wired correctly, and the shared-code touches are safe defaults.

Posting as a comment (non-blocking) rather than requesting changes, since nothing here is a demonstrated defect — just spots that deserve explicit verification against real checkpoints given how much downstream correctness depends on them.

@kiddyboots216
kiddyboots216 force-pushed the codex/oss-fdn-05-dsv4-glm5-20260731 branch from ae9cefe to e556408 Compare July 31, 2026 21:10
@kiddyboots216
kiddyboots216 force-pushed the codex/oss-fdn-05-dsv4-glm5-20260731 branch from e556408 to 1530ce9 Compare July 31, 2026 21:22
@kiddyboots216
kiddyboots216 force-pushed the codex/oss-fdn-05-dsv4-glm5-20260731 branch from 1530ce9 to 0b499ee Compare July 31, 2026 21:29
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