Skip to content

[benchmarking] Add configurable GPU memory warning threshold#1966

Merged
rlratzel merged 6 commits into
NVIDIA-NeMo:mainfrom
rlratzel:gpu_warning_threshold
May 29, 2026
Merged

[benchmarking] Add configurable GPU memory warning threshold#1966
rlratzel merged 6 commits into
NVIDIA-NeMo:mainfrom
rlratzel:gpu_warning_threshold

Conversation

@rlratzel

@rlratzel rlratzel commented May 11, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Adds gpu_mem_use_warning_threshold at session and per-entry level so GPU memory warnings only fire when usage exceeds a configured fraction of total GPU memory (0.0-1.0)
  • Threshold is applied both before and after each benchmark run, with context-specific suffixes ("used before benchmark started" vs "left in use after benchmark ended")
  • Threshold values are validated at construction time to be in the range [0, 1]; out-of-range values raise ValueError
  • Collects warnings from both checks into result_data["warnings"] so sinks (e.g. SlackSink) read them directly instead of re-deriving them from raw GPU stats
  • SlackSink decoupled from GPU warning logic

Test plan

  • Run benchmarks with gpu_mem_use_warning_threshold set in the session config and confirm warnings only fire when GPU usage exceeds the threshold
  • Confirm pre-run and post-run warnings render with their respective suffixes
  • Confirm out-of-range values (e.g. -0.1, 1.5) raise ValueError at session/entry construction
  • Confirm result_data["warnings"] is populated and appears correctly in the Slack sink message
  • Confirm no warnings are emitted when GPU memory usage is below the threshold
  • Run without the field set and confirm behavior is unchanged (any usage > 0 triggers a warning)

🤖 Generated with Claude Code

Adds `max_allowed_gpu_mem_use_warning_threshold` at session and entry
level so pre-run GPU warnings only fire when memory usage exceeds a
configured fraction of total GPU memory. Warnings are collected from
both pre- and post-run checks and stored in `result_data["warnings"]`
so sinks (e.g. SlackSink) can read them directly instead of
re-deriving them from raw GPU stats.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Signed-off-by: rlratzel <rratzel@nvidia.com>
@copy-pr-bot

copy-pr-bot Bot commented May 11, 2026

Copy link
Copy Markdown

Auto-sync is disabled for draft pull requests in this repository. Workflows must be run manually.

Contributors can view more details about this message here.

rlratzel and others added 2 commits May 29, 2026 08:42
- Rename `max_allowed_gpu_mem_use_warning_threshold` to
  `gpu_mem_use_warning_threshold` (less wordy; "threshold" already implies
  a limit).
- Add a `warning_threshold_msg` parameter to `log_gpu_stats` so callers
  can tag warnings with a context-specific suffix; `run.py` now uses
  "used before benchmark started" pre-run and "left in use after
  benchmark ended" post-run.
- Post-run check now also honors the threshold (was previously bare),
  matching the pre-run behavior so small residuals don't trip warnings.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Signed-off-by: rlratzel <rratzel@nvidia.com>
@rlratzel rlratzel marked this pull request as ready for review May 29, 2026 15:00
@greptile-apps

greptile-apps Bot commented May 29, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR adds a configurable gpu_mem_use_warning_threshold (fraction 0.0–1.0) that controls when GPU memory-use warnings fire, replacing the previous hard-coded "any usage > 0" trigger. Warnings are now computed in run.py (both pre-run and post-run) and stored in result_data["warnings"], letting sinks like SlackSink read them directly instead of re-deriving them from raw GPU stats.

  • Threshold validation is applied at construction time in both Session.__post_init__ and Entry.__post_init__; out-of-range values raise ValueError. Entries that don't set their own value inherit the session-level threshold.
  • log_gpu_stats now returns a list[str] of triggered warning messages and handles the None threshold case as legacy behaviour (warn on any usage > 0).
  • SlackSink is decoupled from GPU warning logic and reads result_dict.get("warnings", []) with a safe fallback for cases where the key is absent (e.g., failed runs).

Confidence Score: 5/5

The change is well-scoped: validation is enforced at construction for both session and entry, inheritance is correct, and the SlackSink falls back gracefully when warnings are absent.

All mutations are confined to the benchmarking subsystem. The threshold validation covers both Session and Entry, the None-threshold legacy path is preserved correctly, and the SlackSink decoupling uses a safe .get() fallback. No data paths were broken.

No files require special attention.

Important Files Changed

Filename Overview
benchmarking/runner/utils.py log_gpu_stats now accepts a threshold and returns collected warning strings; logic is correct and handles the None (legacy) case properly
benchmarking/run.py Both pre-run and post-run calls to log_gpu_stats now pass the entry threshold and accumulate warnings into result_data["warnings"]
benchmarking/runner/entry.py Adds gpu_mem_use_warning_threshold field with [0, 1] range validation in post_init
benchmarking/runner/session.py Adds session-level gpu_mem_use_warning_threshold with validation and propagation to entries that have not set their own value
benchmarking/runner/sinks/slack_sink.py Decoupled from GPU warning logic; now reads pre-computed warnings from result_dict["warnings"] with a safe .get() default
benchmarking/nightly-benchmark.yaml Adds session-level gpu_mem_use_warning_threshold: 0.01 with a correctly-worded "Fraction (0.0-1.0)" comment

Sequence Diagram

sequenceDiagram
    participant run_entry
    participant log_gpu_stats
    participant result_data
    participant SlackSink

    run_entry->>log_gpu_stats: "gpu_stats_before, warn_if_in_use=True,<br/>warning_threshold=entry.gpu_mem_use_warning_threshold,<br/>msg="used before benchmark started""
    log_gpu_stats-->>run_entry: warnings[] (pre-run)

    run_entry->>run_entry: run_command_with_timeout(cmd)

    run_entry->>log_gpu_stats: "get_gpu_stats(), warn_if_in_use=True,<br/>warning_threshold=entry.gpu_mem_use_warning_threshold,<br/>msg="left in use after benchmark ended""
    log_gpu_stats-->>run_entry: warnings[] (post-run)

    run_entry->>result_data: "update({..., "warnings": pre+post warnings})"

    run_entry->>SlackSink: register_benchmark_entry_finished(result_data)
    SlackSink->>result_data: get("warnings", [])
    SlackSink-->>SlackSink: render warning lines in Slack message
Loading

Reviews (3): Last reviewed commit: "[benchmarking] Document warning_threshol..." | Re-trigger Greptile

Comment thread benchmarking/nightly-benchmark.yaml Outdated
Comment thread benchmarking/runner/utils.py
Comment thread benchmarking/runner/session.py Outdated
Update the YAML comment and the session docstring to make clear that the
`gpu_mem_use_warning_threshold` is applied to GPU memory usage checks
both before and after each benchmark run, not just before.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Signed-off-by: rlratzel <rratzel@nvidia.com>
Adds the missing docstring entry for the `warning_threshold_msg` argument
introduced in an earlier commit on this branch.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Signed-off-by: rlratzel <rratzel@nvidia.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants