Skip to content

fix: use population std for native Tinker advantages - #1115

Merged
bradhilton merged 2 commits into
mainfrom
tarkin/tinker-native-population-std
Oct 10, 2026
Merged

bradhilton merged 2 commits into
mainfrom
tarkin/tinker-native-population-std

Conversation

@bradhilton

@bradhilton bradhilton commented Oct 9, 2026 •

Copy link
Copy Markdown
Collaborator

Agent: Tarkin

Part of #1111 (item 10).

Problem: tinker_native normalized advantages with the sample standard deviation (n−1), while ART's other backends use the population standard deviation (n). Its advantages were therefore smaller by √((n−1)/n), 0.87 at n = 4. Equal rewards also weren't caught: FP32 rounding left a mean residue that the std guard didn't treat as zero, so [0.3] * 8 produced advantages of +0.94 with normalization on, and about 3e-8 with it off.

Behavior after this change:

  • Advantages use the population standard deviation, matching the other backends. Existing tinker_native runs will see advantages scaled up by √(n/(n−1)).
  • Groups whose rewards agree within 1e-12 get advantages of exactly 0, with normalization on or off. That's the same tolerance the pipeline trainer uses.

Validation: New tests/unit/test_tinker_native_advantages.py pins the population std convention, degenerate and equal-reward groups, the existing 1e-8 std guard, and the unnormalized path. 7 of its 12 cases fail on main. The tinker_native suites pass.

Limitations: The 1e-12 tolerance is inlined here. If #1118 merges first, this should switch to its shared rewards_have_zero_variance helper.

@bradhilton
bradhilton force-pushed the tarkin/tinker-native-population-std branch 2 times, most recently from c6cda82 to d379c4a Compare October 10, 2026 17:35
@bradhilton
bradhilton force-pushed the tarkin/tinker-native-population-std branch from d379c4a to edb3c9a Compare October 10, 2026 18:58
@bradhilton
bradhilton marked this pull request as ready for review October 10, 2026 19:26
@bradhilton
bradhilton merged commit e7f46b4 into main Oct 10, 2026
12 checks passed
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.

1 participant