Repository navigation
fix(split-mtp): commit outputs from private staging - #2277
Merged
Merged
Conversation
Write every generated artifact into a private sibling staging directory with exclusive file creation, then atomically publish new output directories or replace only validated drafter files under --force. Refuse non-empty destinations by default, protect foreign weights and symlinked targets, and stream safetensors with byte-equivalent ordering. Add concurrency, symlink, permissions, serializer, and CLI regression coverage. Refs #1784
inureyes
added
area:cli
Document the atomic output design, path security review, focused regressions, and pinned GLM byte-identity acceptance in English and Korean. Refs #1784
Merge main at fc45284 so PR #2277 uses the maintained safe-Rust YAML parser and no longer inherits the RUSTSEC-2024-0449 cargo-deny failure. The split-mtp source remains unchanged. Validation: configured version, kernel, Llama, ROCm, Python, and WebUI contracts pass; cargo deny check, formatting, and diff checks pass. Refs #1784, #2279
Merge origin/main at 2cb4e20 after the portable A/B output directory fix landed. This incorporates the #2209 and #2276 prerequisite changes while leaving the split-MTP implementation and regression-test paths unchanged. Validation: - configured repository contracts - cargo deny check - cargo fmt --all -- --check - staged and unstaged whitespace checks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
split-mtpnow writes every artifact into a private sibling staging directory and publishes a new or empty output with one directory rename. Non-empty outputs require--force; foreign weights, symlinked destinations, source aliases, and path-resolution changes remain protected.The exclusive streaming safetensors writer preserves the canonical byte layout without allocating a second multi-gigabyte serialization buffer.
Related issues
Closes #1784
Type of change
feat— new user-visible featurefix— bug fixperf— performance improvement (include before/after numbers in the PR body)refactor— internal restructuring without behavior changechore— build, CI, dependencies, release infrastructuredocs— documentation onlytest— tests onlyTest plan
cargo fmt --all -- --checkcargo clippy --bin mlxcel --profile test-fast --features cuda -j2 -- -D warningscargo test -p mlxcel-surgery --lib --profile test-fast -j2 --no-run split_mtp; focused binary: 26 passedcargo test --bin mlxcel --profile test-fast --features cuda -j2 split_mtp: 10 passedzai-org/GLM-4.7-Flashrevision7dd20894a642a0aa287e9827cb1a1f7f91386b67,--q-bits 4, 54 tensors and 15 quantized tensors. Baseline and candidate outputs match byte for byte: weights84018ae89ff88bf727f2552523748a21acb4e824d7f22bcf77d8a439d5f4dd61, config7218072d09e5bb06e3eb005b2481ea0b1da08e7e7cd9ad59c69dbd132a754c24.Notes for reviewers
Fresh and empty outputs use one atomic directory rename. Forced replacement validates the destination again, then renames only the known drafter files individually, as specified by the issue. Staging directories use mode
0700on Unix and clean themselves up after every error.The focused serializer regression compares mixed dtypes and multiple tensor names byte for byte against
safetensors::serialize_to_file. Path regressions cover.,.., Unix non-UTF-8 names, dangling links, concurrent winners, ancestor-symlink retargeting, and unresolved source aliases.CI note:
cargo-denyreports the existing main-branchunsafe-libyaml 0.2.11maintenance advisory (RUSTSEC-2024-0449) throughserde_yaml. This PR changes no dependency or advisory policy and adds no ignore.Checklist
feat:,fix:, etc.)// Used by: ...comments on shared functions modified here.envfiles committed