fix(minimax-h3): materialize TP column shards - #1390
Conversation
2292893 to
16e3a5f
Compare
|
V100 TP4 integration results for the current head ( @gushiqiao, could you please review this TP shard storage-ownership fix when convenient? It is adjacent to the MiniMax-H3 TP checkpoint-loading path covered by #1353. The current GitHub Actions suite is @codex review |
|
Codex Review: Didn't find any major issues. Swish! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
Summary
cloneWhy
torch.chunk(..., dim=0)returns a contiguous view. Calling.contiguous()on that view does not allocate new storage, so a rank-local CPU shard can keep the complete checkpoint tensor backing storage alive.The local shard now owns only its expected storage. This does not change shard values, shapes, dtypes, non-TP loading, or inference numerics.
Upstream alignment
Rebased onto current upstream
mainatf8aee98b5462cca8d7288888146ebd95592bf266after MiniMax-H3 AdaLN caching landed in #1413. The shard fix remains independent and applies cleanly.Validation
pre-commit run --all-files_select_tensor_parallel_shardV100 TP4 integration validation
Validated PR head
16e3a5fce94ad14b66c12c463c0fc2f67d69959don four Tesla V100 PCIe 32 GB GPUs with a full MiniMax-H3 workload (864x480, 124 frames, 20 evaluations). The downstream V100 runtime uses the exact loader expression introduced by this PR:Results were consistent across all four ranks:
40,225,668,128bytes (37.463 GiB) per rank before local materialization10,543,386,144bytes (9.819 GiB) per rank, or26.21%of source storage; the excess over exactly 25% comes from intentionally replicated tensors9.840 GiBper rank14,078 MiBper GPUOOMKilled=false; corrected and uncorrected volatile ECC remained 0This confirms that each TP rank owns its local shard instead of retaining the complete 37.46 GiB source checkpoint storage.
Scope note: the end-to-end run used a downstream branch that also contains separate V100 compatibility work. This validates the identical shard-materialization path on V100; it does not claim that this PR alone provides all changes required for complete V100 inference.