Skip to content

[MFU] Use device peak flops instead of hardcoded H100's - #7197

Open
dwarez wants to merge 5 commits into
huggingface:mainfrom
dwarez:feat/sys-metrics
Open

dwarez wants to merge 5 commits into
huggingface:mainfrom
dwarez:feat/sys-metrics

Conversation

@dwarez

@dwarez dwarez commented Sep 13, 2026

Copy link
Copy Markdown

What does this PR do?

The async trainers currently compute MFU using the compute_mfu function but without passing it the device's flops, therefore the formula will use the default value for it, which is the peak flops for the H100.

In this PR we define _PEAK_FLOPS_BY_DEVICE which maps the device name to its peak flops (I used torchtitan's mapping as the main source of truth and validated most of nvidia's entries with the official specs). Furthermore, the mapping is dtype/precision aware so it can be eventually expanded with lower precision types.

I preferred to avoid exposing the mfu computation when the peak flops could not be retrieved since using a generic denominator would provide false data to the user, but let me know if you'd like this behavior to be different.

I tested these changes using hf jobs with 2xH200 for both trainers on some smoke runs (so the mfu is quite low) but the retrieved peak flops was indeed correct

Before submitting

  • This PR fixes a typo or improves the docs (you can dismiss the other checks if that's the case).
  • Did you read the contributor guideline, Pull Request section?
  • Was this discussed/approved via a GitHub issue? Please add a link to it if that's the case.
  • Did you make sure to update the documentation with your changes?
  • Did you write any new necessary tests?

AI writing disclosure

We welcome the use of AI tools to help with contributions. For transparency and to help us improve our review process, please indicate the level of AI involvement in this PR.

  • No AI usage: the PR was written entirely by a human.
  • AI-assisted: some parts were suggested or improved by AI, but the PR was written and reviewed by a human.
  • AI-generated: the PR was mostly or fully generated by an AI tool.

Who can review?

Anyone in the community is free to review the PR once the tests have passed. Feel free to tag members/contributors who may be interested in your PR.


Note

Low Risk
Changes are limited to observability (MFU metrics) and utility lookups; training loss and optimization paths are untouched, with MFU omitted rather than misreported when capacity is unknown.

Overview
MFU in AsyncGRPO and AsyncDistillation no longer assumes a fixed H100 peak FLOPs denominator. At trainer init, peak capacity is resolved from the actual training devices and mixed-precision dtype via new helpers get_peak_flops and get_peak_flops_per_device in trl/trainer/utils.py, backed by a dtype-aware _PEAK_FLOPS_BY_DEVICE table (NVIDIA, AMD, Trainium/Inferentia, TPU). compute_mfu now requires an explicit peak_flops_per_device (the old H100 default is removed).

perf/mfu_fwd_bwd and perf/mfu_wall_clock are logged only when every rank has a known peak for that precision; otherwise MFU is skipped and throughput/timing metrics still run. Heterogeneous clusters use the mean peak across ranks when all are known.

Docs for both async trainers expand the Performance section (throughput vs MFU, _fwd_bwd vs _wall_clock, training-device-only scope, omission behavior). Tests cover the lookup table, distributed “all ranks must be known” rule, and step-metric MFU gating.

Reviewed by Cursor Bugbot for commit e658593. Bugbot is set up for automated code reviews on this repo. Configure here.

Values grabbed from Torchtitan + expanded with some instances available
in hf jobs
fix: device name check is now bounded to avoid collisions (like A100 and
A1000)
add: mfu infos in trainers docs
@bot-ci-comment

Copy link
Copy Markdown

The docs for this PR live here. All of your documentation changes will be reflected on that endpoint. The docs are available until 30 days after the last update.

Signed-off-by: DWarez <dario.salvati@huggingface.co>
@AmineDiro
AmineDiro self-requested a review September 14, 2026 08:13
Comment thread trl/trainer/utils.py

# Theoretical dense accelerator throughput. Values and sources follow TorchTitan's BF16 peak-FLOPs lookup, extended
# with the additional NVIDIA GPUs offered by Hugging Face Jobs. More specific names must precede their prefixes.
_PEAK_FLOPS_BY_DEVICE = (

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Niice, I wanted to have this in utils for a long time 😄 !
bf16 is enough for now I think, most people will train in this precision

Comment thread trl/trainer/utils.py Outdated
device_name = torch.xpu.get_device_name(device)
else:
device_name = device.type
dtype = {"bf16": torch.bfloat16, "fp16": torch.float16, "no": model_dtype}.get(accelerator.mixed_precision)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we should probably pass the dtype

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

right that makes sense, I changed it and also changed the mapping (the dict associated to the accelerator name) to reflect the same naming (so without torch.) to make matching more immediate 👍

Comment thread tests/test_utils.py Outdated
("NVIDIA L40", torch.bfloat16),
("NVIDIA A1000", torch.bfloat16),
("NVIDIA XA100", torch.bfloat16),
("Intel Data Center GPU Max 15500", torch.bfloat16),

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am really not sure trl works with this device in the general settings.
We use NCCL groups and I am pretty just this vllm weightransfer is Nvidia only 🤔

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah I wasn't really sure about that. I saw that TRL has explicit support for XPUs but I guess that not everything works out of the box. I've removed those entries for the time being, we can also add them back eventually

@AmineDiro AmineDiro left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for fixing this! 🤗 Just small nits

change: removed intel stuff from the lookup because of we're not sure
it's supported, so removing it for now

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Want higher recall? High effort reviews run extra passes and find more bugs. A team admin can switch effort levels in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit daf8010. Configure here.

Comment thread trl/trainer/utils.py
Comment thread trl/trainer/utils.py
Comment on lines +1786 to +1799
device = accelerator.device
if device.type == "cuda":
device_name = torch.cuda.get_device_name(device)
else:
device_name = device.type
peak_flops = get_peak_flops(device_name, dtype)
peaks = gather_object([peak_flops])
if any(peak is None for peak in peaks):
logger.info(
"MFU metrics are disabled because the peak FLOPs are unknown for at least one training device or "
"precision. Throughput and timing metrics are still reported."
)
return None
return sum(peaks) / len(peaks)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is too defensive: mixed GPU models in a single job basically never happens, and even when it does nothing breaks, log builds the metrics dict locally and only the main process reports, so ranks disagreeing about whether to emit perf/mfu_* doesn't hang anything.
I'd drop the gather and just use the local device's peak. Same for the device.type != "cuda" branch, xla devices report device.type == "xla" so it never matches the TPU or trn entries in the table.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can I use torch.cuda.get_device_name(accelerator.device) or do we need a check also for TPUs? In that case I can drop tranium entries from the mapping. In case we cannot map we just warn and then omit the metric

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants