Skip to content

[ckpt] feat: stage Megatron HF safetensors writes when a local dir is set - #7992

Closed
tongyx361 wants to merge 5 commits into
verl-project:mainfrom
tongyx361:tyx/safetensors-staging-opt-in
Closed

tongyx361 wants to merge 5 commits into
verl-project:mainfrom
tongyx361:tyx/safetensors-staging-opt-in

Conversation

@tongyx361

@tongyx361 tongyx361 commented Sep 22, 2026 •

Copy link
Copy Markdown
Collaborator

What does this PR do?

Megatron HF safetensors saves write each shard directly on the destination filesystem. safetensors >= 0.8 serialize_file (safetensors#764) preallocates with File::set_len and then rename/chmods a sibling temp file. HDFS FUSE returns ENOSYS from set_len (safetensors#787). mountpoint-s3 fails that rename/chmod with ENOSYS or EPERM (safetensors#792). This PR adds an opt-in local POSIX staging directory so serialize_file runs on local storage and the finished shard is copied onto the destination with os.replace. That final replace still requires a same-directory rename, so a mount that rejects rename entirely, including mountpoint-s3, is not a supported destination. null or a blank path keeps the direct write. Ordinary local disks stay on that default. Any non-empty path enables staging; the installed safetensors version is not consulted.

This is one independent slice of #7570. That combined PR is being closed in favor of this change plus the separate rollout PRs.

Checklist Before Starting

Duplicate-work check: open staging PRs such as #7869 (delta staging lifecycle), #4770 (PEFT key normalization), and #7286 (FSDP map_location) do not add safetensors_staging_dir. #7570 is the combined predecessor this slice is split out of, and it is being closed rather than left open beside this PR.

Test

python3 -m pytest -q \
  tests/utils/ckpt/test_megatron_checkpoint_manager_on_cpu.py::test_save_hf_via_bridge_skips_staging_when_dir_blank \
  tests/utils/ckpt/test_megatron_checkpoint_manager_on_cpu.py::test_save_hf_via_bridge_stages_serialize_file

2 passed (6.62s) on Python 3.12 with the local Megatron and safetensors stack. The blank-dir case writes the destination path in place. The set-dir case serializes under the staging directory.

pre-commit run --show-diff-on-failure --color=always --files \
  tests/utils/ckpt/test_megatron_checkpoint_manager_on_cpu.py \
  verl/trainer/config/_generated_ppo_megatron_trainer.yaml \
  verl/trainer/config/actor/megatron_actor.yaml \
  verl/trainer/config/critic/megatron_critic.yaml \
  verl/utils/checkpoint/megatron_checkpoint_manager.py \
  verl/workers/config/checkpoint.py

All 14 configured hooks passed, including autogen-trainer-cfg. scripts/generate_trainer_config.sh reports the generated Megatron trainer yaml is current.

GPU saves onto HDFS FUSE, where safetensors >= 0.8 File::set_len returns ENOSYS, were not run. CPU tests do not establish that save.

API and Usage Example

actor_rollout_ref.actor.checkpoint.safetensors_staging_dir=/scratch/hf
# or critic.checkpoint.safetensors_staging_dir=/scratch/hf

Set this only when the checkpoint directory cannot finish safetensors >= 0.8 serialize_file, and point it at a local POSIX directory on the same node. HDFS FUSE is the known case: File::set_len returns ENOSYS (safetensors#787). Leave the value null on ordinary local disks. Any non-empty path enables staging. The installed safetensors version is not consulted. safetensors#792 is the same 0.8 writer failing on mountpoint-s3 rename/chmod; that mount also rejects the same-directory os.replace used to publish the copied shard, so it is not a supported destination.

Design & Code Changes

  • The direct-write path stays the default. Staging exists so a save can succeed when the destination cannot finish safetensors >= 0.8 serialize_file (safetensors#764): HDFS FUSE returns ENOSYS from File::set_len (safetensors#787). A mount that cannot rename at all, such as mountpoint-s3 (safetensors#792), is outside this workaround because the copy still uses os.replace. This does not replace mbridge_config.distributed_filesystem.
  • McoreCheckpointConfig.safetensors_staging_dir defaults to null and is exposed on the Megatron actor and critic checkpoint configs. The generated Megatron trainer yaml is updated to match.
  • _stage_safetensors_writes patches safetensors.torch.serialize_file for the duration of _save_model_as_hf_via_bridge. megatron-bridge binds save_file at import time, and save_file looks up serialize_file on each call, so patching save_file would miss the bridge path.
  • Each shard is written under a temporary directory in the staging root, then copied onto the destination via a same-directory temp file and os.replace, so a crashed copy does not leave a truncated shard.
  • The serializer patch is restored on success or failure. A blank or whitespace-only directory is treated as unset.

Checklist Before Submitting

  • Read the Contribute Guide.
  • Apply pre-commit checks to the PR files. All 14 hooks passed.
  • Add / Update the documentation. When to set the directory is documented on McoreCheckpointConfig.safetensors_staging_dir. Actor and critic yaml comments and the save helpers link there.
  • Add unit tests in the existing tests/utils/ckpt/test_megatron_checkpoint_manager_on_cpu.py module. cpu_unit_tests.yml already collects tests/**/test_*_on_cpu.py.
  • Once your PR is ready for CI, send a message in the ci-request channel. GPU CI is not requested yet.
  • This PR does not touch the recipe submodule.

AI assistance (Cursor) was used to split this slice out of #7570 and prepare the PR. Human review of every changed line remains the submitter's responsibility.

… set

Destination filesystems that reject safetensors allocation calls can opt in with safetensors_staging_dir. The default remains a direct write.

Co-authored-by: Cursor <cursoragent@cursor.com>
tongyx361 and others added 4 commits September 22, 2026 20:01
Users need to know the direct write fails on some shared, network, and mounted filesystems that reject safetensors allocation calls.

Co-authored-by: Cursor <cursoragent@cursor.com>
Staging is for destinations that reject safetensors >= 0.8 serialize_file, such as HDFS FUSE set_len. Mounts that cannot rename at all are outside the final os.replace.

Co-authored-by: Cursor <cursoragent@cursor.com>
The filesystem cases stay on McoreCheckpointConfig.safetensors_staging_dir. Actor and critic yaml comments and the save helpers link there.

Co-authored-by: Cursor <cursoragent@cursor.com>
@wuxibin89 wuxibin89 closed this Sep 23, 2026
@tongyx361

Copy link
Copy Markdown
Collaborator Author

Closing this. Pinning safetensors to 0.7 avoids the failure this staging path was written for, so the workaround does not need to land in verl.

serialize_file in safetensors >= 0.8 (safetensors#764) preallocates with File::set_len (ftruncate). HDFS FUSE returns ENOSYS from that call and the save aborts (safetensors#787). safetensors 0.7 does not do that preallocation, so a direct write to the same destination succeeds. Downgrade with:

pip install 'safetensors==0.7.*'

The proper fix is still on the safetensors side (make set_len best-effort). This PR only papers over the 0.8 writer, and the final os.replace still cannot publish onto a mount that rejects rename entirely.

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