docs: Update callback code snippets to include all imports needed for example#2283
Conversation
|
/ok to test 7777851 |
📝 WalkthroughWalkthroughDocumentation examples in Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
No actionable comments were generated in the recent review. 🎉 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 |
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
docs/training/callbacks.md (1)
3-3:⚠️ Potential issue | 🟡 MinorFix typo in intro sentence.
“propietary” → “proprietary”.
🤖 Fix all issues with AI agents
In `@docs/training/callbacks.md`:
- Around line 67-82: The example uses undefined callbacks (MyCallback,
TimingCallback, MetricsCallback) which breaks copy‑paste; update the code block
to include minimal in‑block callback definitions or replace their usages with
simple inline callbacks that implement the expected interface used by
CallbackManager (e.g., define a small class MyCallback with the relevant hook
methods or add lambdas/anonymous functions) so the snippet from CallbackManager,
manager.register(...), and pretrain(config, forward_step, callbacks=manager) is
fully runnable without external imports.
| ```python | ||
| from megatron.bridge.training.callbacks import CallbackManager | ||
| from megatron.bridge.training.gpt_step import forward_step | ||
| from megatron.bridge.training.pretrain import pretrain | ||
| from megatron.bridge.recipes.qwen import qwen25_500m_pretrain_config | ||
|
|
||
| manager = CallbackManager() | ||
| manager.add(MyCallback()) | ||
| manager.add([TimingCallback(), MetricsCallback()]) | ||
| manager.register("on_eval_end", lambda ctx: print("Evaluation complete!")) | ||
|
|
||
| pretrain(config, forward_step_func, callbacks=manager) | ||
| # Create a config that fits on a single GPU | ||
| config = qwen25_500m_pretrain_config() | ||
|
|
||
| pretrain(config, forward_step, callbacks=manager) | ||
| ``` |
There was a problem hiding this comment.
Mixing example still isn’t copy‑paste runnable.
This snippet references MyCallback, TimingCallback, and MetricsCallback without definitions/imports in the block. Given the PR objective (“directly runnable when copy/pasted”), please either add minimal class definitions in this block or replace with fully in‑block example callbacks.
🤖 Prompt for AI Agents
In `@docs/training/callbacks.md` around lines 67 - 82, The example uses undefined
callbacks (MyCallback, TimingCallback, MetricsCallback) which breaks copy‑paste;
update the code block to include minimal in‑block callback definitions or
replace their usages with simple inline callbacks that implement the expected
interface used by CallbackManager (e.g., define a small class MyCallback with
the relevant hook methods or add lambdas/anonymous functions) so the snippet
from CallbackManager, manager.register(...), and pretrain(config, forward_step,
callbacks=manager) is fully runnable without external imports.
Signed-off-by: Ananth Subramaniam <ansubramania@nvidia.com>
7777851 to
91d28d7
Compare
|
/ok to test 91d28d7 |
… example (#2283) Signed-off-by: Ananth Subramaniam <ansubramania@nvidia.com> Signed-off-by: NeMo Bot <nemo-bot@nvidia.com>
… example (NVIDIA-NeMo#2283) Signed-off-by: Ananth Subramaniam <ansubramania@nvidia.com> Signed-off-by: sowmen <sowmendipta@gmail.com>
What does this PR do ?
Update callback docs to be directly runnable if copy/pasting the code snippet
Changelog
GitHub Actions CI
See the CI sectionin the Contributing doc for how to trigger the CI. A Nvidia developer will need to approve and trigger the CI for external contributors.
Before your PR is "Ready for review"
Pre checks:
If you haven't finished some of the above items you can still open "Draft" PR.
Additional Information
Summary by CodeRabbit