-
Notifications
You must be signed in to change notification settings - Fork 2.9k
Add contributor skills at .agents/skills
#6901
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
qgallouedec
wants to merge
8
commits into
main
Choose a base branch
from
contributor-skills
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
8 commits
Select commit
Hold shift + click to select a range
6310fc6
Add contributor skills at .agents/skills
qgallouedec 1a8f286
Merge branch 'main' into contributor-skills
qgallouedec 29e35e3
Merge branch 'main' into contributor-skills
qgallouedec c9004ff
Merge branch 'main' into contributor-skills
qgallouedec 1eac00e
Merge branch 'main' into contributor-skills
qgallouedec 0d828eb
Point the agent setup at the tracked .agents/skills
qgallouedec 591b106
Merge branch 'main' into contributor-skills
qgallouedec 05e0f16
Merge branch 'main' into contributor-skills
qgallouedec File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,42 @@ | ||
| --- | ||
| name: check-trainer-consistency | ||
| description: Keep duplicated trainer code aligned across TRL trainers. Use when modifying or reviewing code in any trainer (GRPO, RLOO, SFT, DPO, ...) that also exists in sibling trainers, e.g. generation paths, reward computation, metric logging, or weight syncing. | ||
| --- | ||
|
|
||
| # Check trainer consistency | ||
|
|
||
| Trainers in TRL are **self-contained by design**. Shared logic (vLLM generation paths, `_get_per_token_logps_and_entropies`, `_calculate_rewards`, `_prepare_inputs`, metric logging, weight syncing) is deliberately duplicated across trainers instead of being abstracted into a base class, so each trainer stays readable and evolvable in isolation. | ||
|
|
||
| The tradeoff: duplication is accepted, but **consistency is mandatory**. | ||
|
|
||
| ## Rules for duplicated blocks | ||
|
|
||
| - Same variable names (`self._last_loaded_step`, `self._metrics[mode]`, ...). | ||
| - Same control-flow structure (if/elif/else branches in the same order). | ||
| - Same comments, word-for-word when the logic is identical. | ||
| - Divergences only where the trainer's semantics require it (e.g. GRPO extracts logprobs from vLLM, RLOO discards them). | ||
|
|
||
| **Consistency over correctness.** When duplicating code, reproduce it exactly — even if you believe the original has a bug. Do not silently fix the issue in your copy: keep it consistent and report the problem so it can be fixed across all trainers in one dedicated PR. A consistently-wrong codebase can be fixed in a single sweep; an inconsistent one cannot. | ||
|
|
||
| ## When modifying duplicated code | ||
|
|
||
| 1. Identify the duplicated block you are changing. | ||
| 2. Find every copy. Grep a distinctive line of the block across the main and experimental trainers: | ||
| ```sh | ||
| grep -rn "self._last_loaded_step" trl/trainer/ trl/experimental/ | ||
| ``` | ||
| 3. Apply the same change to every copy. A fix in GRPO usually implies the same fix in RLOO and vice versa. Not propagating a change is a bug. | ||
| 4. Verify the copies stayed aligned by diffing the corresponding regions, e.g.: | ||
| ```sh | ||
| diff <(sed -n '/def _generate_single_turn/,/def /p' trl/trainer/grpo_trainer.py) \ | ||
| <(sed -n '/def _generate_single_turn/,/def /p' trl/trainer/rloo_trainer.py) | ||
| ``` | ||
| Remaining diffs must all be semantic divergences, not drift. | ||
|
|
||
| ## When reviewing | ||
|
|
||
| If a PR touches duplicated logic, check that all copies were updated consistently. The most common mistake is fixing one trainer and forgetting the others. | ||
|
|
||
| ## Scope | ||
|
|
||
| Main code (`trl/trainer/`) must stay stable and consistent. Experimental code (`trl/experimental/`) may lag; small non-invasive alignment improvements are welcome, large refactors are not. | ||
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,36 @@ | ||
| --- | ||
| name: update-paper-index | ||
| description: Add or review a paper entry in TRL's paper index. Use when a PR implements a method, algorithm, or training approach from a research paper, or when reviewing such a PR. | ||
| --- | ||
|
|
||
| # Update the paper index | ||
|
|
||
| Any PR that implements a method, algorithm, or training approach from a research paper must add a corresponding subsection to `docs/source/paper_index.md`. When reviewing such a PR, check that the file was updated. | ||
|
|
||
| ## Entry format | ||
|
|
||
| The file is organized as one `##` section per method family (usually one per trainer), each holding `###` subsections, one per paper. | ||
|
|
||
| Each entry contains: | ||
|
|
||
| 1. The paper title as the `###` heading. | ||
| 2. A paper link line, using the Hugging Face paper page (same ID as arXiv), never an arxiv.org link: | ||
| ``` | ||
| **📜 Paper**: https://huggingface.co/papers/<id> | ||
| ``` | ||
| 3. A few sentences on what the paper introduces and how it maps to TRL. | ||
| 4. A Python snippet showing the TRL config that reproduces the paper's setting, with the paper's hyperparameters quoted in comments: | ||
| ```python | ||
| from trl import GRPOConfig, GRPOTrainer | ||
|
|
||
| training_args = GRPOConfig( | ||
| beta=0.001, # "the KL coefficient to 0.001" | ||
| num_generations=16, # "For each question, we sample 16 outputs..." | ||
| ) | ||
| ``` | ||
| When the paper doesn't specify hyperparameters, say so in a comment rather than inventing values. | ||
|
|
||
| ## Placement | ||
|
|
||
| - If the paper belongs to an existing method family, add it under that `##` section. | ||
| - If it introduces a new trainer or family, add a new `##` section with a one-line pointer to the trainer, e.g. `Papers relating to the [`GRPOTrainer`].` |
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
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
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
Oops, something went wrong.
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.
Uh oh!
There was an error while loading. Please reload this page.