Skip to content

fix(checkpoint): FTW truncated shard raises OSError instead of silent garbage load - #25

Merged
jason-fxz merged 2 commits into
FlashML-org:mainfrom
RuixiangMa:fix-ftw-silent-corruption
Aug 24, 2026
Merged

fix(checkpoint): FTW truncated shard raises OSError instead of silent garbage load#25
jason-fxz merged 2 commits into
FlashML-org:mainfrom
RuixiangMa:fix-ftw-silent-corruption

Conversation

@RuixiangMa

@RuixiangMa RuixiangMa commented Aug 22, 2026

Copy link
Copy Markdown
Contributor

Problem

_pread_into in ftw.py silently broke out of its read loop on EOF (os.preadv returning 0), leaving the destination buffer partially filled with uninitialized garbage. A truncated FTW shard file (from interrupted conversion, disk full, or corruption) would load partial weights with no error anywhere, producing silently corrupted model outputs.

Fix

  • O_DIRECT path (_pread_into): break → raise OSError with bytes read/expected and offset.
  • mmap fallback (FTWReader.read_into): added length check before slice assignment with shard file name diagnostics. Previously raised a cryptic ValueError.
  • Unified exception type to OSError across both paths.
  • Updated docstring to reflect current behavior.

cc @andy-yang-1 @jason-fxz

@gdevenyi

Copy link
Copy Markdown

Triaged while assembling a merged deployment branch for a 2× RTX 6000 Ada / 2× Xeon Gold 6526Y Linux box serving DeepSeek-V4-Flash with offloaded experts, to benchmark the open PRs together.

FTW truncated-shard error handling. Not merged: this deployment serves HF safetensors directly and never exercises the FTW reader, so I would have been testing nothing. The change reads correctly — turning a bare OSError into a diagnosable message is the right instinct, and it is the same class of problem as the pin failure in #56.

Flagging only so the absence of a report from me is not read as a problem found — I merged and benchmarked #30, #48, #56, #69, #70, #71 and #81, and left this one out deliberately.

@RuixiangMa RuixiangMa changed the title [Checkpoint][Bugfix]: FTW truncated shard raises OSError instead of silent garbage load Aug 23, 2026
@jason-fxz

Copy link
Copy Markdown
Collaborator

ftw.py looks good! Just drop the tests and ready to merge.

@RuixiangMa

Copy link
Copy Markdown
Contributor Author

ftw.py looks good! Just drop the tests and ready to merge.

Done

@jason-fxz
jason-fxz merged commit bd372b6 into FlashML-org:main Aug 24, 2026
@andy-yang-1

Copy link
Copy Markdown
Collaborator

@RuixiangMa Great job!

avlp12 pushed a commit to avlp12/FreeToken that referenced this pull request Aug 24, 2026
Brings in from FlashML-org/FreeToken main (5 commits since our merge base):

  bd372b6 fix(checkpoint): FTW truncated shard raises OSError instead of
          silent garbage load (FlashML-org#25)
  e0a3bbc Merge pull request FlashML-org#112 from FlashML-org/feat/split-residency
  831d38a fix(moe): report the residency banks actually settle at
  eebb3f5 feat(moe): estimate expert-bank bytes from the model config
  c41833b feat(moe): per-layer host-bank residency -- split lock-CPU/pin-GPU
          layers, auto selection under capped pin quota
  184a4f1 build(kernel-cache): add sm_80 (A100/A800) to default arches (FlashML-org#75)

Conflicts (all in files where upstream's split-residency work and our
own MoE/offload patches touched the same lines) were resolved to keep
both intents, not to pick a side:

* moe/host_banks.py: upstream's split-residency HostBank (backing="cuda"
  born-pinned option, LOCKED/PinPipeline/ResidencyPlan machinery) merged
  with our explicit 4096-byte alignment invariant (_off/_reg_len, aligned
  memoryview()/pin()). The alignment carve now applies to BOTH the mmap
  and the cuda backing, not just mmap. Also fixed a latent bug the
  combination would otherwise have introduced: HostBank.lock() mlock'd
  len(self._buf) (the over-allocated padded buffer) starting at the
  aligned self.addr, which overruns the mmap by up to one block; now
  mlocks self._reg_len like pin() already did.
* moe/expert_banks.py: our bank-alignment startup diagnostic
  (_log_bank_alignment/_ALIGN) and upstream's parallel-reader support
  flag (_PARALLEL_READER_SUPPORTED) are independent additions next to
  each other -- kept both. In load_expert_banks, kept upstream's
  requested_residency()/_echo_residency() wrapping and added our
  _log_bank_alignment(banks) call right after the build, before echoing
  residency back to the caller.
* models/deepseek_v4/moe.py: kept both prefill_routed guards -- our
  speculative-verify-wearing-a-prefill's-clothes check (route to
  _decode_routed under hybrid decode) runs first, then upstream's
  crossover-size-or-unpinned-layer check (route to
  super()._prefill_routed) -- both gate independently to their own
  fallback path.
* moe/offload_cache.py: in copy_missing(), reordered so the unpinned
  (LOCKED) whole-layer pageable materialize check (upstream) runs before
  the --moe-copy-engine DMA doorbell dispatch (ours): an unpinned layer
  has no device alias, so it must never reach the doorbell path, which
  assumes a pinned bank layout.
* engine/engine.py: took upstream's version of the cpu_layer_ids
  resolution -- it's a strict superset of ours (adds _auto_cpu_layers
  selection under a capped pin budget, which is the feature this sync
  exists to pull in); our `n_moe` local was just an unused-elsewhere
  alias, nothing of ours was lost.

Also (per the task): rewrote sync-upstream.sh to `git merge` instead of
`git rebase` -- uran-custom already carries its own merge commit
(pr69/pr70/pr71), and rebasing across an existing merge commit invites
history corruption.

Verified in worktree /root/ft-sync (PYTHONPATH override, not the live
editable install at /root/FreeToken):
- python3 -m compileall python/freetoken: clean
- import freetoken + the 7 touched modules: clean
- /root/test_dma_doorbell.py (eager + graph phases): PASS
- /root/test_bank_alignment.py: PASS
- /root/test_moe_copy_engine_gate.py: PASS
- /root/test_dma_dead_flag.py: PASS
- ad hoc HostBank backing="cuda" (born-pinned) + lock() mlock-length fix
  smoke test: PASS

Our uran-specific features (host_kv_tier.py, DmaCopyService + gating,
alignment invariant, --moe-collect-stats, scheduler idle hooks) are
unaffected outside the files above and behave identically.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Samfisheryu pushed a commit to Samfisheryu/FreeToken that referenced this pull request Aug 26, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

4 participants