[Sync][rebuild] FlashQLA/GDN (#1947) + param-buffer cpu-backup (#1952) + DistOptim step ckpt rider (#1945) - #155
Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces support for the FlashQLA backend as an optional GDN implementation for Qwen3.5 and Qwen3-Next models, including Dockerfile integration, documentation, command-line arguments, and tests. It also updates Megatron patches to support disabling parameter buffer CPU backups, refactors memory checks in reloadable process groups to skip certain communication operations, and refactors model offloading/switching logic. A review comment suggests improving the robustness of the version parsing utility in qwen_gdn_backend.py to handle non-numeric characters in version strings (such as pre-releases) using regular expressions.
| import torch | ||
|
|
||
|
|
||
| def _parse_version(version): | ||
| version = version.split("+", 1)[0] | ||
| parts = version.split(".") | ||
| major = int(parts[0]) | ||
| minor = int(parts[1]) if len(parts) > 1 else 0 | ||
| return major, minor |
There was a problem hiding this comment.
The _parse_version function uses a simple string split on . to parse version numbers. This can fail with a ValueError if any version component contains non-numeric characters (e.g., pre-releases or release candidates like 12.8rc1 or 2.8a0). Using a regular expression to extract digit sequences is much more robust and prevents potential runtime crashes.
| import torch | |
| def _parse_version(version): | |
| version = version.split("+", 1)[0] | |
| parts = version.split(".") | |
| major = int(parts[0]) | |
| minor = int(parts[1]) if len(parts) > 1 else 0 | |
| return major, minor | |
| import re | |
| import torch | |
| def _parse_version(version): | |
| digits = re.findall(r'\d+', version) | |
| major = int(digits[0]) if len(digits) > 0 else 0 | |
| minor = int(digits[1]) if len(digits) > 1 else 0 | |
| return major, minor |
422e92f to
219d443
Compare
…istributedOptimizer step ckpt rider (#1945) Mirror slime @cutoff 7a7aba4 (slime/->vime/, slime_plugins/->vime_plugins/): - #1947: Dockerfile FlashQLA pip; vime_plugins/models/qwen_gdn_backend.py (fla|flashqla selector); --qwen-gdn-backend arg; qwen3_5/qwen3_next GDN selector + contiguous(flashqla); reloadable_process_group comm memory-check skip; tests (linear_attn + memory_check); install doc. N/A vs slime: docker/Dockerfile.gb10 + build_conda.sh (vime ships neither). - #1952: megatron.patch disable_param_buffers_cpu_backup (param_and_grad_buffer + DDP + get_model); drop --disable-weights-backuper + enable_weights_backuper usages (single_tag=None); arguments.py validate/role wiring; placement_group.py deepcopy + critic flag. - #1945 rider (megatron.patch only): DistributedOptimizer 'step' checkpoint handling (del tensors['step'], step LocalNonpersistentObject, src_tensors.pop) — engine-agnostic, rode along with #1952 in the same megatron.patch (slime authored #1952 on top of #1945). All rebuild-gated: take effect only after image rebuild. py_compile OK on all edits. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
219d443 to
fec799e
Compare
PR #155 synced #1945's DistOptim checkpoint rider but missed the gpu_lock_exec.py change: replaces os.execvp with subprocess.Popen + proper signal forwarding (SIGINT/SIGTERM/SIGHUP) and fd_lock cleanup. This prevents orphaned GPU-holding processes when CI runners are cancelled. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…ms (#218) * fix: complete slime #1985 sync — remove TIGHT_HOST_MEMORY + align params PR #214 synced #1985's TIGHT_DEVICE_MEMORY removal and batch-size shrinks but missed 10 files that used TIGHT_HOST_MEMORY (a different env-var guard with the same pre-#1985 pattern). Also aligns n-samples-per-prompt (8→4) and num-critic-only-steps (3→2) that were left at pre-#1985 values. Files: test_moonlight_16B_A3B{,_r3}, test_qwen3_{0.6B_parallel_check, 30B_A3B{,_r3}, 4B_ckpt, 4B_ppo{,_disaggregate,_train_critic_only}, 4B_streaming_partial_rollout}, test_qwen2.5_0.5B_ppo_critic_only_short Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: remove leftover blank lines from TIGHT_ variable removal #214 and the preceding commit removed TIGHT_DEVICE_MEMORY and TIGHT_HOST_MEMORY definitions but left behind an extra blank line in the header area of 11 test files. Collapse double-blank to single-blank to match slime's spacing. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * revert: remove pre-ported slime #2016 content from http_utils http_utils.py had get_rollout_num_engines() and run_router improvements that were pre-ported from slime #2016 (post-cutoff 44d29ee). Revert to match slime@44d29ee baseline so the diff stays clean; #2016 will be synced as a whole when its turn comes. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * chore: remove test_vllm_generate_endpoint.py (vime-native, no slime counterpart) This test was vime-specific (created in #49, not from slime) and was never registered in CI. Remove it to keep the test tree aligned with slime@44d29ee. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * sync: port gpu_lock_exec subprocess signal forwarding (slime #1945) PR #155 synced #1945's DistOptim checkpoint rider but missed the gpu_lock_exec.py change: replaces os.execvp with subprocess.Popen + proper signal forwarding (SIGINT/SIGTERM/SIGHUP) and fd_lock cleanup. This prevents orphaned GPU-holding processes when CI runners are cancelled. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: complete #1985 sync + fix NUM_GPUS placement + pre-commit - Remove TIGHT_HOST_MEMORY from 10 test files (missed in #214) - Align n-samples-per-prompt (8→4) and num-critic-only-steps (3→2) - Remove leftover blank lines from TIGHT_ variable removal (11 files) - Revert http_utils.py pre-ported #2016 content to slime@44d29ee baseline - Remove test_vllm_generate_endpoint.py (vime-native, no slime counterpart) - Port gpu_lock_exec subprocess signal forwarding (slime #1945) - Align remaining test params (over-sampling-batch-size, max-tokens-per-gpu) - Full test_qwen3_4B_ckpt.py #1945 sync (optimizer placement CLI) - Fix NUM_GPUS=0 placement in 7 CPU test files (was inside decorators) - NamedTemporaryFile multi-line formatting All pre-commit checks pass. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Summary
Rebuild-gated sync PR (touches
docker/+megatron.patch→ needs image rebuild to validate). Three engine-agnostic slime PRs, mirror-slime:--qwen-gdn-backend fla|flashqla): Dockerfile FlashQLA pip (arch-gatedARG INSTALL_FLASHQLA, x86=1/arm=0 — sm90/Hopper-only, verified gb200/Blackwell import-fails + downgrades tilelang) +qwen_gdn_backend.py+ qwen3_5/qwen3_next selector + reloadable_process_group comm memory-check skip + tests + install doc.disable_param_buffers_cpu_backup(train-side tms, engine-agnostic): megatron.patch param-buffer cpu-backup + drop--disable-weights-backuper+ arguments/placement wiring.stepckpt rider (engine-agnostic): megatron.patchdel tensors['step']+ step LocalNonpersistentObject + src_tensors.pop. (sglang v0.5.12 body = N/A; vime has no sglang image.)Validation
Both images built (x86 FLASHQLA=1 + arm FLASHQLA=0), megatron.patch applies clean both arches. See #107.
🤖 Generated with Claude Code