fix(megatron): restore "compute per GPU" TFLOP label and make log parsers robust - #926
Open
amd-fuyuajin wants to merge 6 commits into
Open
fix(megatron): restore "compute per GPU" TFLOP label and make log parsers robust#926amd-fuyuajin wants to merge 6 commits into
amd-fuyuajin wants to merge 6 commits into
Conversation
The Flux MLPerf logging refactor (#820) reverted the TFLOP label from "compute per GPU (TFLOP/s/GPU): X (avg Y)" back to the ambiguous "throughput per GPU (TFLOP/s/GPU): X/Y" while extracting the shared _inject_tflops helper. Restore the clearer "compute" wording (per #842) so the FLOP rate is not confused with the tokens/s throughput line, and so the code matches the current examples/odc docs. Also refresh the render-format docstrings to match the actual output. Co-authored-by: Cursor <cursoragent@cursor.com>
The training-log patch now renders "compute per GPU (TFLOP/s/GPU): X (avg Y)" and "tokens/s/GPU inst/harmonic mean: X/Y". Update the three log scrapers (daily_report, auto_benchmark/metrics, benchmark_report) to accept both these current labels and the legacy "throughput per GPU (...): X/Y" / "tokens per GPU (tokens/s/GPU): X/Y" forms, so old and new logs both parse. Label/separator alternations use non-capturing groups to keep positional group indices stable. In daily_report the average is now optional and falls back to the instantaneous value for raw (un-averaged) Megatron lines. Co-authored-by: Cursor <cursoragent@cursor.com>
The training log now appends memory last as "hip mem usage/free/total/usage_ratio: X GB/..." (after tokens) rather than "mem usages: X" between elapsed and throughput. Match the memory segment at the end of the line in its current position, accept both the hip-mem and legacy "mem usages:" labels (capturing only the used value), and reorder the extracted group indices accordingly. Co-authored-by: Cursor <cursoragent@cursor.com>
Real Megatron pre-train logs render the compute average as "X(avg Y)" (the space before "(avg" can be dropped by the log sink), which the previous " \(avg " literal did not match. Make the space optional so benchmark_report parses real logs, matching the tolerance already present in daily_report and auto_benchmark/metrics. Co-authored-by: Cursor <cursoragent@cursor.com>
amd-fuyuajin
requested review from
Xiaoming-AMD,
limou102 and
wenxie-amd
as code owners
July 27, 2026 22:32
Black joins the two comment-adjacent implicitly-concatenated regex fragments onto one line (line-length 110). No functional change; the concatenated pattern string is identical. Co-authored-by: Cursor <cursoragent@cursor.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Restores the
compute per GPU (TFLOP/s/GPU)training-log label (from #842) thatwas inadvertently reverted by the Flux MLPerf logging refactor (#820), and updates
the three Megatron log parsers so both the current and legacy log formats parse
correctly.
Background
throughput per GPU (TFLOP/s/GPU): X/Yto
compute per GPU (TFLOP/s/GPU): X (avg Y). "Throughput" is ambiguous next tothe tokens/s line;
computedescribes a FLOP rate unambiguously._inject_tflops()helper for the diffusion path and, inthe process, reverted that label back to
throughput per GPU. The harmonic-meantoken change from improve the logging format for the megatron backend #842 was left intact.
examples/odc/README.md, added after feat(flux): MLPerf logging/warmup/lr-schedule patches #820) already document thelog as
compute per GPU (TFLOP/s/GPU), so the code had drifted from the docs.What changed
primus/backends/megatron/patches/training_log/print_rank_last_patches.pycompute per GPU (TFLOP/s/GPU): X (avg Y)label in_inject_tflops().(
compute per GPU ...andtokens/s/GPU inst/harmonic mean: ...).Log parsers —
tools/daily/daily_report.py,tools/auto_benchmark/metrics.py,benchmark/megatron/model/benchmark_report.pycompute per GPU,tokens/s/GPU inst/harmonic mean)and the legacy ones (
throughput per GPU,tokens per GPU (tokens/s/GPU)), usingnon-capturing alternations so positional group indices are unchanged.
benchmark_report: the memory segment is now appended last (aship mem usage/free/total/usage_ratio: X GB/...) rather than sitting betweenelapsed and throughput as
mem usages: X; reorder the pattern/group indices andaccept both memory labels.
(avg(real logs can renderX(avg Y)), andfall back to the instantaneous value when a running average is absent (raw
warmup lines).
Why the parsers were already broken
The token label from #842 (
tokens/s/GPU inst/harmonic mean) never matched theparsers'
tokens per GPU (tokens/s/GPU)regex, andbenchmark_reportexpected amem usages:segment that current logs no longer emit. This PR realigns all threewith the current log format while staying backward-compatible.
Test plan
pytest tests/unit_tests/backends/megatron/test_training_log_patches.py— 7 passedThroughputAverageExtension/ElapsedAverageExtensionacross multiple iterations, rendered the line, and confirmed all three parsers
extract correct inst/avg TFLOP, tokens, and memory values.
(
llama2_7B-BF16-pretrain, MI300X): current + legacy + raw-warmup lines all parse.