Skip to content

feat: add throughput/prompt_length/total_num_tokens metrics #781

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.

Sign up for GitHub

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

Jump to bottom
Merged
terrykong merged 3 commits into from
Aug 2, 2025
Merged

feat: add throughput/prompt_length/total_num_tokens metrics #781

terrykong merged 3 commits into from
Aug 2, 2025

Conversation

@ZhiyuLi-Nvidia
Copy link
Contributor

@ZhiyuLi-Nvidia ZhiyuLi-Nvidia commented Jul 29, 2025 •
edited
Loading

What does this PR do ?

Add a one line overview of what this PR aims to accomplish.
As attached. Add the following metrics:

  • train/mean_prompt_length: mean token length of prompt
  • train/total_num_tokens: total num of tokens (including prompt and generation tokens)
  • train/throughput_per_gpu: num of tokens per second (including prompt and generation tokens)

https://wandb.ai/nvidia/grpo-dev/runs/33pb40kk?nw=nwuserzhiyul

As a quick verification/test:

  • train/total_num_tokens = num_prompts_per_step x num_generations_per_prompt x (train/mean_prompt_length + train/mean_total_tokens_per_sample)
    • actual: 2869598 vs calculated 2869596.16 (numerical difference)
  • train/throughput = train/total_num_tokens / step time / total_num_gpus
    • actual: 1199 vs 1199

Issues

List issues that this PR closes (syntax):

Usage

  • You can potentially add a usage example below
# Add a code snippet demonstrating how to use this

Before your PR is "Ready for review"

Pre checks:

  • Make sure you read and followed Contributor guidelines
  • Did you write any new necessary tests?
  • Did you run the unit tests and functional tests locally? Visit our Testing Guide for how to run tests
  • Did you add or update any necessary documentation? Visit our Document Development Guide for how to write, build and test the docs.

Additional Information

  • ...

@ZhiyuLi-Nvidia
feat: add throughput/prompt_length/total_num_tokens metrics
df774bd 
Signed-off-by: Zhiyu Li <zhiyul@nvidia.com>
@ZhiyuLi-Nvidia ZhiyuLi-Nvidia requested review from SahilJain314, parthchadha and terrykong and removed request for parthchadha July 29, 2025 08:07
@ZhiyuLi-Nvidia
lint
a55649d 
Signed-off-by: Zhiyu Li <zhiyul@nvidia.com>
parthchadha
parthchadha reviewed Jul 29, 2025
View reviewed changes
@snowmanwwg snowmanwwg mentioned this pull request Jul 29, 2025
Closed
@snowmanwwg snowmanwwg linked an issue Jul 29, 2025 that may be closed by this pull request
No built-in tokens/second metric #588
Closed
@ZhiyuLi-Nvidia
Copy link
Contributor Author

ZhiyuLi-Nvidia commented Jul 30, 2025

@terrykong @SahilJain314 could you take a look?

parthchadha
parthchadha reviewed Jul 30, 2025
View reviewed changes
terrykong
terrykong reviewed Jul 30, 2025
View reviewed changes
@ZhiyuLi-Nvidia
rename throughput metrics
707a7d2 
Signed-off-by: Zhiyu Li <zhiyul@NVIDIA.com>
@ZhiyuLi-Nvidia ZhiyuLi-Nvidia force-pushed the zhiyul/add_throughput_metric branch from 345388b to 707a7d2 Compare July 31, 2025 22:00
@terrykong terrykong added this pull request to the merge queue Aug 1, 2025
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Aug 1, 2025
@terrykong terrykong added this pull request to the merge queue Aug 1, 2025
Merged via the queue into main with commit 60633f4 Aug 2, 2025
15 checks passed
@terrykong terrykong deleted the zhiyul/add_throughput_metric branch August 2, 2025 01:36
tpoisonooo pushed a commit to tpoisonooo/RL that referenced this pull request Aug 4, 2025
@ZhiyuLi-Nvidia @tpoisonooo
feat: add throughput/prompt_length/total_num_tokens metrics (NVIDIA-N…
2fb7f84 
…eMo#781)

Signed-off-by: Zhiyu Li <zhiyul@nvidia.com>
Signed-off-by: Zhiyu Li <zhiyul@NVIDIA.com>
Signed-off-by: tpoisonooo <khj.application@aliyun.com>
soodoshll pushed a commit to soodoshll/RL that referenced this pull request Aug 13, 2025
@ZhiyuLi-Nvidia @soodoshll
feat: add throughput/prompt_length/total_num_tokens metrics (NVIDIA-N…
f0bfbfe 
…eMo#781)

Signed-off-by: Zhiyu Li <zhiyul@nvidia.com>
Signed-off-by: Zhiyu Li <zhiyul@NVIDIA.com>
Signed-off-by: Qidong Su <qidongs@nvidia.com>
PrinsYin pushed a commit to PrinsYin/RL that referenced this pull request Nov 30, 2025
@ZhiyuLi-Nvidia
feat: add throughput/prompt_length/total_num_tokens metrics (NVIDIA-N…
05d3e63 
…eMo#781)

Signed-off-by: Zhiyu Li <zhiyul@nvidia.com>
Signed-off-by: Zhiyu Li <zhiyul@NVIDIA.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@terrykong terrykong terrykong left review comments

@parthchadha parthchadha parthchadha approved these changes

@SahilJain314 SahilJain314 Awaiting requested review from SahilJain314

Assignees

@ZhiyuLi-Nvidia ZhiyuLi-Nvidia

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

No built-in tokens/second metric

4 participants

@ZhiyuLi-Nvidia @terrykong @parthchadha