Repository navigation
Remove code left unreachable by the legacy engine removal - #254
Merged
Merged
Conversation
michaellchung
force-pushed
the
s23-pr-12-secrlenv-ports
branch
from
October 14, 2026 11:32
a5c72fb to
defb6cc
Compare
validate_deepseek_v4_bridge_runtime.py and validate_deepseek_v4_clone_lora_runtime.py build their models through the legacy Miles modules (megatron_utils.lora_utils, megatron_utils.trainable_state) that the ports image does not ship, and they assert on legacy-only runtime internals (expert_full_runtime actor/export helpers, clone_lora.assert_original_expert_lora_zero, island_entry megatron_adapter_targets). With the legacy engine removed in 1543c319 they can no longer run, and porting them would mean rewriting them against the ports state plugin rather than adapting them, so they are removed with their unit test. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The legacy Miles engine (removed in 1543c319) was the only caller of expert_full_runtime.install and clone_lora.install. Remove them together with everything only they reached: the legacy trainable_state / actor / actor_group / lora_utils / arguments / model / weight-iterator / update-weight patches, the hybrid trainable-state export, chunked Ray transport and apply lifecycle, the train-telemetry capsule, and clone_lora.assert_original_expert_lora_zero. Kept for ports_runtime: configured_expert_count, export_selected_expert_weights, weight_iterator_task_bridge, wrap_expert_full_optimizer (with its master check), install_or_defer, wrap_clone_only_lora and ClonePackedAdapterPolicy. The V4 Bridge hook now arms only the ports lora.bridge module; the legacy bridge_lora_helpers target cannot exist in a ports image. Tests that exercised only the removed paths are deleted; docstrings no longer describe a legacy layout. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
megatron_adapter_targets (with _pipeline_local_target and its two module regexes) mapped PEFT specs onto Megatron module paths for the legacy engine's LoRA setup. Ports Miles derives LoRA targets itself, and after the legacy engine and the legacy DSV4 validation scripts were removed nothing calls it. Its unit tests and the full-mode "must not call" stub go with it. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The harness accepts only the strict-avg and decoupled sync presets, and engine.selection rejects expert-full tuning on any preset but full-parameter, so _validate_plan always refuses a plan with expert_full_count > 0. Remove the code that could only run after that check: the expert-full field validation, the --expert-full-* learner argv, and the YETO_DSV4_EXPERT_FULL_* container environment. An expanded DeepSeek V4 plan is therefore always clone-only LoRA. prepare still records a requested expert_full_count so validation rejects it with the selection reason instead of dropping the flag silently. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The native arm ran stock Miles' own training loop and left with the legacy engine: benchmark_rl.main already refused it before any run. Drop the arm kind and everything only it reached: the native-checkpoint save, the PEFT normalization of Miles' own adapter, the syncer-less run path, and the delta-vs-native report column. WorkerSpec.policy_sync was False only for the native arm, so workers now always run with Yeto policy sync. An --arms value naming native now fails as an unknown arm. Historical native results in docs/BENCHMARK_RESULTS.md are kept as recorded evidence. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
applied_lr.before_train_step and grad_audit.before_train_step were wired only through the legacy fork's --custom-megatron-before-train-step-hook-path. Ports reads the applied LR and arms the gradient audit from the state plugin's train_one_step wrapper, so the hooks, their HOOK_PATH constants, the legacy applied-LR file side channel (record/collect), the unused summarize helper and grad_audit.legacy_bindings have no caller. optimizer_lr, AppliedLrError, arm, enabled and GradBinding stay for the state plugin. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The plan and run subcommands launched benchmark_rl with --rl-engine legacy next to ports; the legacy engine was removed in 1543c319, so they can no longer produce a comparison. The analyze and fake subcommands stay: analyze re-reads recorded legacy/ports work dirs, and the archived rl-engine-ports evidence (env/tolerance.py) imports this module to re-derive its thresholds, so deleting the whole script would orphan that evidence. A ports-only single arm has nothing to compare against, so no ports-only run mode is added. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The full-parameter evidence test docstring still named rollout_legacy_port as the ports rollout function, two import-isolation tests still listed the deleted legacy.engine module, and session_protocol.md described the old legacy MILES_COMMIT pin. State the current facts instead: the protocol follows MILES_NEXT_COMMIT in yeto/rl/adapters/miles/pins.py. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The import scan missed a removed module named in a string, such as a
Miles hook path ("module:attr") or a dotted path handed to
load_function. Scan code, tests and docs for "adapters.miles.legacy"
and "rollout_legacy_port".
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
michaellchung
force-pushed
the
s23-pr-13-remove-unreachable
branch
from
October 14, 2026 11:32
702c83f to
29d87ca
Compare
michaellchung
marked this pull request as ready for review
October 14, 2026 11:32
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
2 participants
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
Deletes about 8k lines that only the legacy engine could reach (P0-1.7b.4-Q3).
Motivation
PM requirement: converge duplicated and dead code.
What changed
megatron_adapter_targetsin island_entry; expert-full plumbing in the SSH harness.benchmark_rl; legacy before-train-step hooks (applied LR, grad audit).rl_engine_equivalence(analyzeandfakestay for archived evidence).session_protocol.md; a text scan forbids naming the removed modules.Behavior changes / compatibility
benchmark_rl --arms nativenow fails as an unknown arm; historical native results in docs are kept.expert_full_count > 0is rejected with the selection reason (it was already rejected).Tests
Local CPU tests: every test file this PR adds or changes: 257 passed, 1 skipped in 27.52s. Full suite with cargo (Rust syncer tests): 11 failed, the same 11 as on s23-final.
Known expected failures until the ports image is rebuilt for the new Miles pin (P0-1.11 GPU run):
test_rl_ports_image(2),test_rl_backend_identity::test_golden_samples_keep_both_old_hashes_and_carry_the_identity, and 8test_decoupling_goldensamples (golden is regenerated after the rebuild).Depends on
#253.
Not verified on GPU
None of this PR has run on GPU yet. The regression GPU run for the S23 stack is P0-1.11 GPU run (in progress at the time of writing); results will be linked here before merge.
🤖 Generated with Claude Code