Update AGENTS.md with learnings from July 2026 PRs - #140
Open
RobotSail wants to merge 1 commit into
Open
Conversation
New learnings from merged PRs #117, #118, #127, #130, #132, #133, #135 (training_hub) and #114 (mini_trainer): - Fix grpo-verl extra split: [grpo] no longer includes verl - trl 1.x tokenizer→processing_class rename gotcha - Corrected os._exit subprocess hang fix: conditional-only + parent-side join timeout (TRAINING_HUB_JOIN_TIMEOUT env var) - ART 0.5.18 model.train() → backend.train() API change - ART 0.5.18 megatron import stub (sys.meta_path finder) - OSFT meta-device dtype trap (nn.Embedding float32 on meta device) - 4 new past incidents documenting the above Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Co-authored-by: multica-agent <github@multica.ai>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAGENTS.md updates document GRPO installation variants, trl 1.x usage, ART 0.5.18 compatibility and shutdown behavior, related incidents, Megatron stubs, and OSFT dtype handling. ChangesAGENTS.md guidance
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
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
1 participant
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
Updates AGENTS.md with 6 new operational learnings distilled from 11 merged PRs across training_hub and mini_trainer (July 7-29, 2026).
What changed
Corrected factual errors:
[grpo]extra no longer includes verl -- verl moved to[grpo-verl](not on AIPCC index). Updated Quick Commands and Installation sections.os._exitsubprocess hang fix description was outdated -- the fix went through 3 iterations (PRs Fix subprocess hang after GRPO training completes #132/Fix subprocess hang: use join timeout instead of blocking forever #133/Fix multi-iteration GRPO with art 0.5.18 #135). Updated to reflect the current two-layer defense: conditionalos._exiton success + parent-sideproc.join(timeout)withTRAINING_HUB_JOIN_TIMEOUTenv var.New operational gotchas:
tokenizertoprocessing_classrename in SFTTrainer (PR #117)train()fromTrainableModeltoLocalBackend(PR #135)sys.meta_pathfinder (PRs #118, #127, #130)nn.Embeddingdefaults to float32 on meta device regardless oftorch_dtype(mini_trainer PR #114)New past incidents (4):
os._exitfix broke multi-iteration GRPO (3 PRs in 36 hours) -- PRs #132/#133/#135PRs analyzed but no learnings extracted (feature work, no operational gotchas)
Summary by CodeRabbit
processing_classterminology.