Skip to content

[DO NOT MERGE] stacked multi-lora full PR - #2846

Closed
yueming-yuan wants to merge 383 commits into
mainfrom
yueming/multi-lora-v2-stacked
Closed

yueming-yuan wants to merge 383 commits into
mainfrom
yueming/multi-lora-v2-stacked

Conversation

@yueming-yuan

@yueming-yuan yueming-yuan commented Aug 31, 2026 •

Copy link
Copy Markdown
Collaborator

Combined view of the multi-LoRA v2 stack against main. Do not merge this PR; land the component PRs in order.

Part PR Content
0/7 #2839 Retire multi-LoRA v1 and retain slot primitives
1/7 #2840 LoRA model extension, dedicated actor, and selective optimizer stepping
2/7 #2841 Per-batch Tinker losses and per-datum outputs
3/7 #2842 Per-slot checkpoints and atomic directory publication
4/7 #2843 Immutable disk snapshots through WeightPublisher
5/7 #2844 Gateway protocol, scheduling, and tenant ownership
6/7 #2845 Miles runtime and frozen-base serving entry point
7/7 #2849 Example, gateway tests, and launcher snapshots

Tinker training runs through ordered per-model streams and per-slot optimizers. lora/actor.py extends MegatronTrainRayActor with the slot commands; lora/model.py:run_forward_backward extends the shared model execution. The worker spec selects the subclass only for Megatron multi-LoRA actor workers.

Saving sampler weights exports the current adapter into an immutable directory containing tensors, adapter config, and ownership/base-model metadata. Saving neither applies nor discards pending gradients. Engines load the same frozen HF base at startup and load adapter snapshots from disk on demand; sampler saves make no inference-engine calls.

training_utils/weight_update/snapshot_publisher.py reuses the shared HF iterator and checkpoint directory transaction. The multi-LoRA actor initializes the publisher; the general actor initializes the ordinary updater for full-model and single-LoRA training. Checkpoint load/save/export execution errors, including filesystem errors, propagate through TrainerCell and stop the current single-cell service. load_state rejects sampler snapshots before reaching the Megatron checkpoint loader; weights_info retains metadata support for both checkpoint kinds. Checkpoint names are relative symlinks to immutable version directories. The shared writer completes shards and metadata before atomically replacing the public link. A process crash leaves the old or new complete checkpoint reachable; older versions remain on disk for readers that already resolved them. Legacy directory checkpoints remain readable, but overwriting them is unsupported: save under a new name. Successful phases retain their collective barriers; execution errors still propagate to the trainer cell. A TODO records deferred IO failure isolation.

A terminal training failure ends the affected model stream, including queued commands, while completed futures retain their results. Known request-local forward-only and sampling failures preserve healthy training resources; a failed sample leaves its snapshot usable. Futures, leases, deduplication, and unsaved accumulation remain process-local. Automatic training recovery and replay are not implemented.

The combined branch and #2849 have the same head: 5ccde8dcde766c2290b721545ce6f0758e51dae2. Checkpoint publication belongs to #2842, the FT harness correction to #2843, gateway validation and checkpoint path handling to #2844, and documentation/tests to #2849. The changes cascade through #2845 and downstream #3198.

Request completion is centralized in _finish_request; checkpoint utilities are stateless, validation remains in the service, and the dispatcher owns background-task shutdown. The final #3198 tree exactly matches reviewed commit de7a5565e82cb6c89e26174d5a2b26980648e086, including the existing single-LoRA checkpoint changes. All updated branches preserve their previous heads as ancestors.

Validation:

  • pre-commit run --all-files and incremental git diff --check passed on every updated component head. All previous heads remain ancestors; the final Unify HF snapshot and checkpoint writing #3198 tree exactly matches the reviewed implementation.
  • Process-exit probes at both sides of publication lost the public path before the fix; afterward it resolves to the complete old or new version, and a subsequent save succeeds. A parameterized regression is included in the existing [multi-lora v2 7/7] Tinker gateway example and tests #2849 checkpoint test file.
  • Sampler-to-training load probes with and without optimizer state now return a user failure before any backend load, while another tenant continues training. Repeated SAVE → LOAD verifies distinct retained versions and loading from a pinned version.
  • Focused checkpoint continuation passed on 2 H200 GPUs (DP=2, TP/PP/EP=1), using real Megatron DDP, MultiLoRA layers, SlotOptimizer and distributed checkpoint IO. After three Adam steps, slot 0 was saved and restored into a fresh optimizer on slot 1; weights, FP32 masters, Adam moments, parameter-group metadata and step counters matched with zero tolerance before and after the next identical-gradient update. Rounding masters to BF16 and restarting from weights alone were both detected by negative controls.
  • The enabled regression is tests/e2e/lora/test_slot_checkpoint_resume.py (Hopper, LoRA, 2-GPU stage). The tested final tree is Unify HF snapshot and checkpoint writing #3198 fd63c7ec8c41c2e0a61d273f0cd5a3504275ada7, with Megatron 73b54618f7e58e0f25f619bcfecbe2640765475a, Bridge 40b930897717941cfe2bd9806f417e28bf1bfa65, PyTorch 2.13.0+cu130 and Transformer Engine 2.17.0. Checkpoints were on GPFS. pre-commit run --all-files passed.
  • This isolates checkpoint/Adam continuation with identical post-reduction gradients; it does not validate forward/backward numerics, topology changes, MoE/TP/PP layouts, server restart, or failure recovery. Fresh hosted CPU/NVIDIA CI is pending; broad GPU selection remains on Unify HF snapshot and checkpoint writing #3198. The Tinker cookbook and multi-tenant acceptance tests are now enabled under run-ci-multi-lora; their first GPU validation is pending.
  • Launchers and command snapshots are unchanged by these fixes. The prior local launcher gate has not passed (362 passed, 30 failed, 122 errors), including macOS/Linux assumptions and existing launcher mismatches.

ci-megatron-pr: 73b54618f7e58e0f25f619bcfecbe2640765475a
ci-sglang-pr: 02b5e123c490c988d21526d0dd7837fefdd0e1da

Router dependency: install the router wheel and standalone binary from the existing cu130-torch213-x86_64 / cu130-torch213-aarch64 rolling releases in yueming-yuan/miles-wheels, using the normal /tmp/wheels installation path. Both architectures were rebuilt on CPU devboxes from radixark/sgl-router-for-miles@b6342a3515e259bbd1eb4db728035a4efbe4fdf9 and verified to forward return_indexer_topk and lora_backfill_paths. No commit-specific router release is required. The existing pr-2849 Docker image has not been rebuilt by this change.

Tinker GPU CI: add tests/e2e/lora/test_tinker_multi_tenant_gateway.py and enable the cookbook gateway test. Both carry the multi-lora domain, selected by the repository label run-ci-multi-lora, on 8 H200 GPUs. They share Qwen3-4B gateway setup (4 training GPUs, TP2/DP2; 4 sampling GPUs; rank-8 adapters). The multi-tenant client trains four distinct markers concurrently, exports each adapter, and checks loss decrease and its own sampled marker.

Validation: the real CI selector selects exactly both enabled tests for run-ci-multi-lora; mocked entrypoint checks confirm both client commands and launcher cleanup. Full pre-commit passed. GPU convergence, cookbook execution, and end-to-end tenant isolation have not yet been validated.

@claude claude Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Claude Code Review

This repository is configured for manual code reviews. Comment @claude review for a one-time review, or @claude review always to subscribe this PR to a review on every future push.

Tip: disable this comment in your organization's Code Review settings.

@fzyzcjy
fzyzcjy force-pushed the tom/refactor-miles/op13-142 branch from deec6fc to 3aa1645 Compare August 31, 2026 23:34
@fzyzcjy
fzyzcjy requested a review from Rockdu as a August 31, 2026 23:34
@yueming-yuan
yueming-yuan force-pushed the yueming/multi-lora-v2-stacked branch 16 times, most recently from 5e1130b to 1287821 Compare September 1, 2026 05:27
@fzyzcjy
fzyzcjy force-pushed the tom/refactor-miles/op13-142 branch 3 times, most recently from 56a39c1 to 7739862 Compare September 1, 2026 14:34
@yueming-yuan
yueming-yuan force-pushed the yueming/multi-lora-v2-stacked branch from 1287821 to 40e81ab Compare September 1, 2026 19:30
@yueming-yuan yueming-yuan mentioned this pull request Sep 15, 2026
7 of 15 tasks
Comment thread serve_tinker.py
trains_attn=bool(target_modules & {"q_proj", "k_proj", "v_proj", "o_proj"}),
trains_mlp=bool(target_modules & {"gate_proj", "up_proj", "down_proj"}),
trains_unembed="lm_head" in target_modules,
)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

max_tokens_per_datum should come from the trainer limit. the example runs with --max-tokens-per-gpu 8192, but this keeps the 32768 default. get_minimum_num_micro_batch_size() does not split or reject one oversized sequence, so an 8193–32768-token datum is admitted and placed alone in an over-budget microbatch. can we derive this from args.max_tokens_per_gpu and the model context limit, and add a boundary test?

Comment thread serve_tinker.py Outdated
vocab_size=vocab_size,
lora_alpha=args.lora_alpha,
max_lora_rank=args.lora_rank,
trains_attn=bool(target_modules & {"q_proj", "k_proj", "v_proj", "o_proj"}),

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

these booleans collapse partial and model-specific target layouts into a contract tinker cannot represent. for example, --target-modules linear_qkv sets trains_attn=true even though o_proj is not trained, while an mla-only attention layout reports false. can we canonicalize the targets once and reject layouts that are not exactly representable by train_attn, train_mlp, and train_unembed, with tests for partial and mla layouts?

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.

6 participants