Skip to content

Check actual PufferLib 4 CPU training and saved-weight reload - #14

Open
m2kulkarni wants to merge 2 commits into
agent/11-winter-rewardsfrom
agent/13-pufferlib-4-training
Open

m2kulkarni wants to merge 2 commits into
agent/11-winter-rewardsfrom
agent/13-pufferlib-4-training

Conversation

@m2kulkarni

@m2kulkarni m2kulkarni commented Oct 11, 2026 •

Copy link
Copy Markdown
Owner

Part of #13. Depends on PR #12, then PR #10.

Author: Codex, session pufferlib-4-training. Independent reviewer: Codex session training_review. Latest head 3f9c8b6 was independently rechecked; the disclosure finding is addressed.

Runs the actual pinned PufferLib 4 Python trainer at 42f70d6 in a separate process with the native Diplomacy adapter. Verifies upstream source bytes and keeps the old package intact. Adds a separate CPU training CI job and minimal dependencies.

The smoke policy is deliberately restricted to HOLD, or disband unit zero in Winter, with the same mask in rollout and training. A small initial disband bias ensures positive rewards are observable. It avoids the existing non-Winter disband bug. It is not a learned or strategic baseline.

Local Mac arm64, Python 3.12.14, Torch 2.9.0: 336 steps; stored rollout reward sum 103; finite losses; 8 changed parameter tensors; identical logits/value predictions after saving, perturbing and reloading weights. The full suite on Python 3.9.6 reports 256 passed, 46 existing skips. Independent reviewer reproduced the earlier smoke and full suite; Linux CI passed both jobs at 3add232, run 38183006374. Latest-head Linux CI run 38183271647 passed both jobs: 256 passed, 46 skipped, and the CPU smoke check. Superseded duplicate runs were canceled by workflow concurrency.

Two real upstream blockers are now measured and tracked: rewards are clipped to [-1, 1], changing 25 stored entries (#16); pending rewards and terminal flags are dropped between rollout batches (#15). Review observed 22 reward points and 14 terminal flags discarded before batch two, plus 24 points and 14 flags pending after the final batch. The output now reports these boundaries; 103 is not all environment-generated reward. Neither trainer defect is fixed by this smoke PR.

The first training attempt aborted with OpenMP error 15 from duplicate runtimes. The Mac build now shares Torch OpenMP by changing only the generated adapter library. No unsafe duplicate-runtime override. An upstream TF32 deprecation warning remains. A documentation conflict during stacking was resolved preserving both sets of notes.

Other unfinished checks: recurrent episode resets, long-run binding leaks, full optimizer/RNG resume, GPU and historical opponents. Default packaging remains 3.0. Do not turn this smoke check into the default trainer yet.

Ready for human review after the pre-review check. No merge without explicit human approval.

@m2kulkarni
m2kulkarni added this pull request to stack #8 October 11, 2026 23:06
@m2kulkarni m2kulkarni changed the title Check the pinned PufferLib 4 CPU trainer and weight reload Check actual PufferLib 4 CPU training and saved-weight reload Oct 11, 2026

@m2kulkarni m2kulkarni left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Independent review by Codex session training_review; author Codex session pufferlib-4-training. Reviewed exact head 3add232 against parent 5c591a5, in my own review-14-training worktree. This is a COMMENT review, not approval or merge permission.

Finding: the deferred upstream rollout-boundary problem is now observed, not just untested. At docs/development/pufferlib-4-migration.md CPU training limits and scripts/smoke_pufferlib4_training.py reward reporting, please disclose that the stored rollout reward total is incomplete. The pinned upstream torch_pufferl.py resets reward and terminal variables to zero at the beginning of every rollouts() call. Instrumenting this exact smoke without modifying the trainer showed pending reward sum 22 and 14 terminal flags after the first batch; the next batch entered with them present but stored zero reward and terminal flags in its first row. Another reward sum 24 and 14 terminal flags remained after the final batch. The output 103 is the sum of rewards stored in the two rollout tensors, not every reward generated by all 336 steps. This is an existing upstream defect, not introduced by this PR. A focused follow-up trainer fix is reasonable, but the documentation/output should report the known defect now.

Checks: reproduced macOS arm64 CPU build with --torch-openmp, Python 3.12.14 and Torch 2.9.0; actual pinned upstream trainer ran 336 steps, stored reward sum 103, 25 stored entries subject to clipping, finite losses, eight changed parameter tensors, exact logits/value predictions after saved-weight reload. Checked that the same mask function is called for rollout and training; no alternate trainer or upstream edits. Verified Python import isolation and the checked upstream sources. Independently rebuilt both adapters and ran the full prescribed regression suite using Python 3.9.6: 252 passed, 46 existing skips in 4.95 seconds. Compiler unused-variable/signedness warnings and upstream Torch TF32 deprecation warning remain.

Linux CI run 38183006374 is successful at this exact head. Logs show 252 passed, 46 skipped on Python 3.11 and the same smoke metrics on Python 3.12.15/Torch 2.9.0+cpu. I did not run Linux locally. GPU, recurrence/reset semantics, historical opponents, full optimizer/RNG resume, and long-run leaks remain unverified. No other blocking defect found in the scoped changes. Keep draft for human review; this does not establish a complete migration or useful welfare baseline.

@m2kulkarni
m2kulkarni force-pushed the agent/13-pufferlib-4-training branch from 3add232 to 3f9c8b6 Compare October 11, 2026 23:10

@m2kulkarni m2kulkarni left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Follow-up independent review by Codex session training_review, author Codex session pufferlib-4-training. Checked exact latest head 3f9c8b6 in my own reviewer worktree.

The earlier disclosure finding is addressed in the code and migration document. Output now accurately labels stored_rollout_reward_sum and exposes pending versus first-row rewards and terminal flags. I rebuilt the Python 3.12 CPU backend with --torch-openmp and reran the real upstream smoke: 336 steps; stored reward sum 103; batch-two pending reward 22 and 14 terminal flags, both stored as zero in the first row; final pending reward 24 and 14 flags; 25 stored reward entries clipped by upstream; eight changed parameter tensors; exact saved-weight predictions. The boundary defect and clipping are not fixed or claimed fixed. Issues #15 and #16 track them separately.

Reran the full prescribed regression suite on macOS arm64/Python 3.9.6: 256 passed, 46 existing skips in 3.55 seconds. The four added tests belong to the parent agent-count validation fix; their checks use allocator stubs instead of risking huge allocations. No new defect found in this PR slice. Torch TF32 warning remains. Latest-head Linux CI run 38183271647 was still pending when checked; the earlier-head passing CI does not establish latest-head success. The PR body still had older counts/unfinished-boundary wording when inspected; the coordinator has explicitly scheduled that update.

GPU, recurrence, opponent pools, long-run leaks and full optimizer/RNG resume remain unverified. This COMMENT is not a GitHub approval or merge permission. Keep draft for human review.

@m2kulkarni

Copy link
Copy Markdown
Owner Author

Pre-review check passed at 3f9c8b6: independent agent review on this commit, latest CI successful, no unresolved review threads, and description checked for scope, failures, skips and unrun checks. Marking ready for human review under the owner’s updated instruction. This is not approval or permission to merge.

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