-
Notifications
You must be signed in to change notification settings - Fork 167
feat: Add deepseek flops tracker #1250
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
Conversation
Signed-off-by: Guyue Huang <[email protected]>
Signed-off-by: Guyue Huang <[email protected]>
📝 WalkthroughWalkthroughAdds NCCL debug environment variables during MegatronPolicyWorker initialization. Extends FLOPS tracking to support DeepSeekV3Config by importing the config and formula symbol, and adding a new branch in convert_config_to_flops_config to construct a DeepSeekV3-specific FLOPSConfig. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Caller
participant FT as flops_tracker.convert_config_to_flops_config
participant HF as transformers.Config (Qwen2/Qwen3/Llama/DeepSeekV3)
participant FF as flops_formulas
Caller->>FT: convert_config_to_flops_config(config)
FT->>HF: inspect type(config)
alt DeepSeekV3Config (new)
FT->>FT: build FLOPSConfig (DeepSeekV3 fields)
FT->>FF: select formula: deepseekv3
else Qwen2/Qwen3/Qwen3Moe/Llama
FT->>FT: build corresponding FLOPSConfig
FT->>FF: select respective formula
else Unsupported
FT-->>Caller: raise ValueError
end
FT-->>Caller: (FLOPSConfig, formula)
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
nemo_rl/models/policy/megatron_policy_worker.py(1 hunks)nemo_rl/utils/flops_tracker.py(2 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.py
📄 CodeRabbit inference engine (CODING_GUIDELINES.md)
**/*.py: Follow the Google Python Style Guide for all Python code
Target Python 3.12+ for all Python code in NeMo-RL
Indent Python code with 4 spaces; do not use tabs
Python filenames should be snake_case (e.g., some_file.py)
Class names should be PascalCase
Function and method names should be snake_case
Local variable names should be snake_case; if starting with a number, prefix with k (e.g., k_99th_percentile)
Global variables should be UPPER_SNAKE_CASE and prefixed with G_ (e.g., G_MY_GLOBAL)
Constants should be UPPER_SNAKE_CASE
Avoid shadowing variables declared in an outer scope
Initialize all externally visible members of a class in the constructor
For public interfaces used outside a file, prefer docstrings over comments
Use comments mainly for code within a function or interfaces local to a file
Commented-out code must include a nearby comment explaining usage and why it is commented out; otherwise remove before merging
Use Google-style docstrings for classes and functions (Sphinx-parseable)
Avoid using reflection when functionality can be easily achieved without it
Limit except clauses to the smallest specific set of exceptions possible
For duck-typing via try/except, keep the try body minimal and use else for main logic
Add the NVIDIA copyright header (with current year) at the top of all Python files, excluding tests/ and test-only scripts
Files:
nemo_rl/models/policy/megatron_policy_worker.pynemo_rl/utils/flops_tracker.py
nemo_rl/**/*.py
📄 CodeRabbit inference engine (CODING_GUIDELINES.md)
nemo_rl/**/*.py: Do not set non-None configuration defaults in code; YAML is the single source of truth for defaults
Access required config attributes directly (e.g., policy_cfg["precision"]) and assume presence; do not introduce hidden defaults
Express configuration optionality via TypedDict using typing.NotRequired
When adding a new config key to a TypedDict subclass, document the key’s purpose, valid values/types, and recommended default in code
For any class or function decorated with @ray.remote, add '# pragma: no cover' on the class/def line (and on remote functions)
Files:
nemo_rl/models/policy/megatron_policy_worker.pynemo_rl/utils/flops_tracker.py
🧬 Code graph analysis (1)
nemo_rl/utils/flops_tracker.py (2)
nemo_rl/models/policy/utils.py (1)
sliding_window_overwrite(169-195)nemo_rl/utils/flops_formulas.py (2)
FLOPSConfig(21-59)deepseekv3(386-458)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Lint check
- GitHub Check: Post submodule check comment / Comment on PR
- GitHub Check: Post automodel integration comment / Comment on PR
terrykong
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
awesome. thanks @guyueh1 !
could you add a unit test here https://github.com/NVIDIA-NeMo/RL/blob/main/tests/unit/utils/test_flops_counter.py#L23-L30 ?
Signed-off-by: Guyue Huang <[email protected]>
Signed-off-by: Guyue Huang <[email protected]>
|
@terrykong added test for this |
|
closing in favor of #1305 |
Pull request was closed
Signed-off-by: Guyue Huang <[email protected]> Signed-off-by: Guyue Huang <[email protected]>
Signed-off-by: Guyue Huang <[email protected]> Signed-off-by: Guyue Huang <[email protected]> Signed-off-by: NeMo Bot <[email protected]>
What does this PR do ?
Add deepseek v3 flops tracker
Issues
List issues that this PR closes (syntax):
Usage
# Add a code snippet demonstrating how to use thisBefore your PR is "Ready for review"
Pre checks:
Additional Information
Summary by CodeRabbit
New Features
Chores