[Doc] Fix/ppo tutorial - #4102
Conversation
🔗 Helpful Links🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/rl/4102
Note: Links to docs will display an error until the docs builds have been completed. ❌ 1 Unclassified FailureAs of commit dd0b68d with merge base 92a3f7f ( UNCLASSIFIED FAILURE - DrCI could not classify the following job because the workflow did not run on the merge base. The failure may be pre-existing on trunk or introduced by this PR:
This comment was automatically generated by Dr. CI and updates every 15 minutes. |
|
| Prefix | Label Applied | Example |
|---|---|---|
[Algorithm] |
new algo | [Algorithm] Add new RL objective |
[BE] |
BE | [BE] Improve error messages |
[Benchmark] or [Benchmarks] |
Benchmarks | [Benchmark] Add collector benchmark |
[BugFix] |
BugFix | [BugFix] Fix memory leak in collector |
[Example] or [Examples] |
Examples | [Example] Add training script |
[Feature] |
Feature | [Feature] Add new optimizer |
[Doc] or [Docs] |
Documentation | [Doc] Update installation guide |
[Refactor] |
Refactoring | [Refactor] Clean up module imports |
[CI] |
CI | [CI] Fix workflow permissions |
[Test] or [Tests] |
Tests | [Tests] Add unit tests for buffer |
[Trainer] or [Trainers] |
Trainers | [Trainer] Add trainer config |
[Environment] or [Environments] |
Environments | [Environments] Add Gymnasium support |
[Data] |
Data | [Data] Fix replay buffer sampling |
[LLM] |
llm/ | [LLM] Add reward model integration |
[Minor] |
small change | [Minor] Fix typo in error message |
[Performance] or [Perf] |
Performance | [Performance] Optimize tensor ops |
[BC-Breaking] |
bc breaking | [BC-Breaking] Remove deprecated API |
[Deprecation] |
Deprecation | [Deprecation] Mark old function |
[Algorithm] or [Algorithms] |
new algo | [Algorithm] Add new objective |
[Quality] |
Quality | [Quality] Fix typos and add codespell |
[Versioning] |
versioning | [Versioning] Bump release version |
[WIP] |
WIP | [WIP] Draft implementation |
Note: Common variations like singular/plural are supported (e.g., [Doc] or [Docs]).
vmoens
left a comment
There was a problem hiding this comment.
I believe this branch is contaminated: Its description and issue link cover only coding_ppo.py, while the diff also adds PostTrainingLogger, rewrites GRPO/expert-iteration logging, changes accumulation boundaries, and adds 382 logger-test lines.
77d21cc to
5cf32c6
Compare
|
| Prefix | Label Applied | Example |
|---|---|---|
[Algorithm] |
new algo | [Algorithm] Add new RL objective |
[BE] |
BE | [BE] Improve error messages |
[Benchmark] or [Benchmarks] |
Benchmarks | [Benchmark] Add collector benchmark |
[BugFix] |
BugFix | [BugFix] Fix memory leak in collector |
[Example] or [Examples] |
Examples | [Example] Add training script |
[Feature] |
Feature | [Feature] Add new optimizer |
[Doc] or [Docs] |
Documentation | [Doc] Update installation guide |
[Refactor] |
Refactoring | [Refactor] Clean up module imports |
[CI] |
CI | [CI] Fix workflow permissions |
[Test] or [Tests] |
Tests | [Tests] Add unit tests for buffer |
[Trainer] or [Trainers] |
Trainers | [Trainer] Add trainer config |
[Environment] or [Environments] |
Environments | [Environments] Add Gymnasium support |
[Data] |
Data | [Data] Fix replay buffer sampling |
[LLM] |
llm/ | [LLM] Add reward model integration |
[Minor] |
small change | [Minor] Fix typo in error message |
[Performance] or [Perf] |
Performance | [Performance] Optimize tensor ops |
[BC-Breaking] |
bc breaking | [BC-Breaking] Remove deprecated API |
[Deprecation] |
Deprecation | [Deprecation] Mark old function |
[Algorithm] or [Algorithms] |
new algo | [Algorithm] Add new objective |
[Quality] |
Quality | [Quality] Fix typos and add codespell |
[Versioning] |
versioning | [Versioning] Bump release version |
[WIP] |
WIP | [WIP] Draft implementation |
Note: Common variations like singular/plural are supported (e.g., [Doc] or [Docs]).
vmoens
left a comment
There was a problem hiding this comment.
Why? The prints in the next two lines do the exact same thing
5cf32c6 to
dd0b68d
Compare
|
| Prefix | Label Applied | Example |
|---|---|---|
[Algorithm] |
new algo | [Algorithm] Add new RL objective |
[BE] |
BE | [BE] Improve error messages |
[Benchmark] or [Benchmarks] |
Benchmarks | [Benchmark] Add collector benchmark |
[BugFix] |
BugFix | [BugFix] Fix memory leak in collector |
[Example] or [Examples] |
Examples | [Example] Add training script |
[Feature] |
Feature | [Feature] Add new optimizer |
[Doc] or [Docs] |
Documentation | [Doc] Update installation guide |
[Refactor] |
Refactoring | [Refactor] Clean up module imports |
[CI] |
CI | [CI] Fix workflow permissions |
[Test] or [Tests] |
Tests | [Tests] Add unit tests for buffer |
[Trainer] or [Trainers] |
Trainers | [Trainer] Add trainer config |
[Environment] or [Environments] |
Environments | [Environments] Add Gymnasium support |
[Data] |
Data | [Data] Fix replay buffer sampling |
[LLM] |
llm/ | [LLM] Add reward model integration |
[Minor] |
small change | [Minor] Fix typo in error message |
[Performance] or [Perf] |
Performance | [Performance] Optimize tensor ops |
[BC-Breaking] |
bc breaking | [BC-Breaking] Remove deprecated API |
[Deprecation] |
Deprecation | [Deprecation] Mark old function |
[Algorithm] or [Algorithms] |
new algo | [Algorithm] Add new objective |
[Quality] |
Quality | [Quality] Fix typos and add codespell |
[Versioning] |
versioning | [Versioning] Bump release version |
[WIP] |
WIP | [WIP] Draft implementation |
Note: Common variations like singular/plural are supported (e.g., [Doc] or [Docs]).
|
@vmoens Good catch. I wanted to prevent users from accidentally deleting the initialization if they remove the prints. I've just updated the PR to assign the outputs to variables instead, ensuring the forward pass only executes once. |
Description
This PR updates the
coding_ppo.pytutorial to explicitly separate the dummy forward pass (required to initializeLazyLinearmodules) from the print statements. Additionally, a Sphinx.. note::was added to clearly warn users that running the dummy batch is strictly required, preventing them from deleting this initialization logic when adapting the tutorial for their own scripts.Motivation and Context
Users copying the tutorial code frequently removed the
print()statements for the dummy forward passes, unaware that they served as the mandatory initialization step for the underlyingLazyLinearparameters. This led to anUninitializedParametererror later in the PPO execution loop when attempting to disable gradients on the uninitialized modules. By explicitly separating the logic and warning the user, this UX bug is resolved.Closes #3134
Types of changes
What types of changes does your code introduce? Remove all that do not apply:
Checklist
Go over all the following points, and put an
xin all the boxes that apply.If you are unsure about any of these, don't hesitate to ask. We are here to help!