[GLM 5.3 Tests] Check trainer/inference LoRA logprob agreement

[hershg]

LoRA logprob agreement

• Compare trainer and inference on fixed tokens, first at zero-init, then after a deterministic trainer-only LoRA B update.
• Compare against the same updated trainer before and after weight sync. The before-sync check must detect missing sync; the after-sync check must pass.

Check	Comparison	Expected mean logprob difference
A	Zero-init trainer vs inference	< 0.05
B	Update trainer only; compare before sync	> 0.05
C	Sync adapter; compare after sync	< 0.05

Evidence

Columns A/B/C match the checks above. Each value is the mean absolute token-logprob difference between trainer and inference: A uses the zero-init trainer; B/C use the same updated trainer before/after sync. Update magnitude is fixed before the run; the runner does not use an optimizer.

Model / input	A: zero-init logprob diff (< 0.05)	B: logprob diff before sync (> 0.05)	C: logprob diff after sync (< 0.05)	Result
Qwen3-0.6B	0.016145	0.100314	0.020233	Pass
Qwen3-8B	0.023879	0.200157	0.021330	Pass
Qwen3-8B, held-out	0.011256	0.111395	0.013102	Pass
GLM-5.3	0.209653	Not reached	Not reached	Fails A: zero-init disagreement

GLM: 192 scored tokens; p99 diff 2.95165, max 5.83631. Inference base vs zero-init LoRA and each backend's repeat scores matched exactly. The unchanged retry completed scoring; the earlier collective timeout did not recur. The mismatch precedes the deliberate LoRA update, so this is not evidence of a weight-sync failure.

Testing / scope

CPU coverage: alignment, nonfinite scores, stale/wrong weights, insufficient updates, deterministic perturbation, cleanup. Qwen GPU evidence above. No batching, optimizer-stress or learning qualification; aggregate agreement does not prove every adapter target works.

Runner / run(). Receipt revisions: Qwen8 and GLM 1e0eddba; Qwen0.6 e93939e9; held-out ab5c8095. GLM retry: skyrl-glm53-pr2-sidecar-20260909-b, two B300 nodes, 2026-09-09.

[hershg]

Moved active review to hershg#2, which genuinely targets PR1’s branch and shows only the child diff. This closes the misleading combined-diff view, not the work. GitHub cannot target a contributor-fork branch from an upstream PR. After #2181 merges, rebase the child onto upstream main and reopen this PR for upstream landing. No code was merged; native GPU validation remains pending.

[gemini-code-assist]

Code Review

This pull request introduces numerical checks and adapter-only perturbation utilities for GPU LoRA testing, along with standalone Megatron LoRA diagnostics and CPU tests. The reviewer's feedback provides valuable robustness improvements, including wrapping JSON serialization to prevent masking assertion errors, ensuring device alignment during log probability comparisons, generating random noise in float32 to avoid PyTorch compatibility issues on CPU, and casting response mask sums to long integers to prevent float-to-integer comparison issues.

[greptile-apps]

Confidence Score: 5/5

The PR appears safe to merge based on the reviewed changes, with no concrete blocking or independently actionable non-blocking defect identified.

Diagram

sequenceDiagram
    participant R as Diagnostic runner
    participant T as Megatron trainer
    participant S as Inference sampler
    R->>S: Score base model
    R->>T: Publish zero-initialized LoRA
    R->>S: Score zero adapter and repeat
    R->>T: Score zero adapter and repeat
    R->>R: Check initial agreement and noise
    R->>T: Perturb trainable LoRA parameters
    R->>T: Score updated trainer
    R->>S: Score unpublished adapter
    R->>R: Verify sampler remains unchanged
    R->>T: Broadcast updated LoRA
    T->>S: Load skyrl-lora weights
    R->>S: Score published adapter
    R->>R: Check parity, update magnitude, and delta agreement

Loading

[cursor]

Cursor Bugbot has reviewed your changes using default effort and found 2 potential issues.

^{Reviewed by Cursor Bugbot for commit f9b1b1c. Configure here.}

[j316chuck]

Two remaining changes before this is a correctness check:

1. Add a localized-corruption regression and gate tail error. With 192 positions, set zero/stale=-2.0, trainer_updated=-1.9 everywhere, and updated=trainer_updated except one position at -10.9. Today check_updated_adapter(report, 0.05) passes because mean_abs=0.046875, despite max_abs=9.0. Add this as a failing regression, then require an explicit max_abs budget (for initial and updated parity) so one bad token cannot hide in the mean.

2. Run publication through the public Tinker sync path. Make the live runner use tinker.ServiceClient -> create_lora_training_client -> save_weights_and_get_sampling_client, apply a fixed forward_backward + optim_step, score the existing sampler while stale, then call save_weights_and_get_sampling_client again and score the published model. Reuse skyrl/tinker/sdk_logprobs.py. The direct broadcast_to_inference_engines call bypasses WorkerDispatch.save_weights_for_sampler behavior such as adapter/model-id selection and weight-version/cache handling.

Already resolved on current head:

3. The isolated pytest command now installs pytest with --with pytest.
4. Native trainer and sampler paths reject NaN/Inf before inserting scores into the JSON receipt, with regression coverage for both.

[j316chuck]

Tentative approval based on a live Qwen/Qwen3.5-4B Megatron run against exact head 1e0eddb (TP4 trainer + 2xTP2 vLLM) through the public Tinker SDK path.

With one lr=1e-3 optimizer update, all 192 probe scores were finite: initial trainer/sampler mean_abs=0.01869, repeat and withheld-publication drift=0, stale mean_abs=1.099, and re-published mean_abs=0.00562. Image digest: sha256:fe8bc62b34b2cb28a5a42a39fa07cb6085e5fdd22dc658b2eb0004193f121de5; deployment XID: 1052900.

Caveat: a deliberately large 2x lr=1e-2 update exceeded the 0.05 absolute-parity budget (mean_abs=0.185, max_abs=1.172) despite cosine=0.99991. Please retain the max/tail-error follow-up; this approval is scoped to the realistic passing update path.