Skip to content

feat(models): add AR value head and GAE wiring - #256

Merged
leviking98z-rgb merged 1 commit into
Tencent-Hunyuan:mainfrom
yhl48:feat/ar-value-head
Aug 1, 2026
Merged

feat(models): add AR value head and GAE wiring#256
leviking98z-rgb merged 1 commit into
Tencent-Hunyuan:mainfrom
yhl48:feat/ar-value-head

Conversation

@yhl48

@yhl48 yhl48 commented Jul 25, 2026

Copy link
Copy Markdown
Contributor

Summary

Part 2/3 of #86: add the critic/value and GAE data path required by AR PPO. This branch has been rebuilt on the current main.

  • Adds an optional scalar FP32 ValueHead for Qwen3.
  • Keeps value-head initialization deterministically zero for both eager construction and meta-init + sharded checkpoint loading.
  • Lets packed and padded Qwen3 replay return aligned per-token values through ReplayResult.values.
  • Adds packed values, returns, and token_advantages fields to TextSegment.
  • Adds terminal-reward scattering and per-trajectory GAE without cross-trajectory leakage.
  • Preserves packed-segment metadata during GAE preparation and treats loss_mask as an optimization mask, not as an artificial trajectory boundary.

The PPO optimizer and trainer wiring remain in #259.

Related Issue

Part of #86. #259 is stacked on this PR and should be reviewed/merged after this one.

Test Plan

No PR-specific test files are included.

  • SKIP=no-commit-to-branch pre-commit run --all-files --show-diff-on-failure — all hooks passed.
  • python -m compileall -q unirl — passed.
  • Temporary, uncommitted correctness harness — passed:
    • eager and meta-init value heads start at exactly zero;
    • meta-init + real sharded checkpoint load matches eager load (max_state_diff=0.0);
    • packed replay values and log-probabilities stay token-aligned;
    • multi-trajectory GAE does not leak across sequence boundaries;
    • reward propagation continues across masked tokens while masked tokens remain excluded from optimization;
    • value-head backward gradient is finite and nonzero.
  • GPU smoke on an H20 with a 9.8M-parameter tiny Qwen3 checkpoint — passed.

Compatibility / Risk

  • use_value_head defaults to false; existing Qwen3 GRPO behavior is unchanged.
  • return_values=false preserves the legacy replay return type.
  • Base Hugging Face checkpoints do not contain critic weights; the new head is initialized from scratch at zero.
  • The value head is train-side only and must be excluded from rollout-engine weight sync; feat(algorithms): add AR PPO with GAE and value loss #259's recipe does this.

Reviewer Notes

Please review packed/padded replay alignment and the meta-init post-load zeroing first. AI assistance was used; the final diff was reviewed and validated against the current repository state.

Checklist

  • I reviewed the changed code and removed unrelated/generated artifacts.
  • I validated the change without adding PR-specific test files.

@github-actions github-actions Bot added the need review Ready and waiting for review label Jul 25, 2026
@yhl48
yhl48 marked this pull request as draft July 25, 2026 16:57
@github-actions github-actions Bot added wip Draft / work in progress and removed need review Ready and waiting for review labels Jul 25, 2026
@yhl48
yhl48 force-pushed the feat/ar-value-head branch from ed4fccd to 719cc08 Compare July 26, 2026 10:38
@yhl48
yhl48 marked this pull request as ready for review July 26, 2026 10:39
@github-actions github-actions Bot added need review Ready and waiting for review and removed wip Draft / work in progress labels Jul 26, 2026
@yhl48
yhl48 force-pushed the feat/ar-value-head branch from 87c5630 to 6728292 Compare July 26, 2026 16:44
@yhl48

yhl48 commented Jul 26, 2026

Copy link
Copy Markdown
Contributor Author

@leviking98z-rgb leviking98z-rgb changed the title feat(models): add AR value head and GAE track wiring feat(models): add AR value head and GAE wiring Aug 1, 2026
@leviking98z-rgb

Copy link
Copy Markdown
Collaborator

Updated this PR on current main in c6aae06c.

Review fixes included:

  • deterministic zero init after meta-init + sharded checkpoint loading;
  • GAE no longer treats loss_mask=0 as a trajectory termination;
  • GAE rebuilding preserves hidden packed-segment metadata;
  • packed and padded Qwen3 replay both return token-aligned FP32 critic values.

Validation result: pre-commit and compileall passed. An uncommitted GPU correctness smoke with a tiny Qwen3 checkpoint confirmed eager/meta loaded states are identical (max_state_diff=0.0), critic values start at zero, replay outputs align, and the value-head gradient is finite and nonzero.

No test files were added.

@leviking98z-rgb

Copy link
Copy Markdown
Collaborator

Added a full-model integration check through the stacked PPO PR #259: Qwen3-4B-Base on 8 x H20, BF16, with both colocated SGLang and trainside rollout.

Three nonzero-gradient PPO rounds completed on both paths. This exercised #256's value head, token-aligned critic replay, terminal reward/GAE preparation, backward, and repeated optimizer updates on the real 4B checkpoint. The loss curves were close:

  • SGLang: -0.2223, -0.2206, -0.2199
  • trainside: -0.2238, -0.2209, -0.2191

All rewards were 0.5000; gradient norms stayed finite and nonzero (16.x), and neither path produced OOM, NaN, exception, or clipping. No test files were added. The detailed SGLang/trainside ratio and log-probability comparison is recorded on #259.

@leviking98z-rgb
leviking98z-rgb merged commit 3a3dae0 into Tencent-Hunyuan:main Aug 1, 2026
8 checks passed
@github-actions github-actions Bot removed the need review Ready and waiting for review label Aug 1, 2026
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