Skip to content

Fix device sections and partial-comparison crash in nvbench_compare.py - #492

Open
MaxFreedomPollard wants to merge 1 commit into
NVIDIA:mainfrom
MaxFreedomPollard:fix/compare-legacy-per-device-states
Open

MaxFreedomPollard wants to merge 1 commit into
NVIDIA:mainfrom
MaxFreedomPollard:fix/compare-legacy-per-device-states

Conversation

@MaxFreedomPollard

Copy link
Copy Markdown
Contributor

nvbench_compare.py (nvbench-compare-legacy) has two bugs in compare_benches(), both caused by the same loop:

  1. Results from more than one GPU are printed wrong. Each ## [id] name section lists the states of all GPUs. Sections get the label of whichever state was processed last, and the summary counts every state once per device. The repo's own sample files show it:

    $ python python/scripts/nvbench_compare.py python/scripts/test_ref.json python/scripts/test_cmp.json
    # simple
    
    ## [1] Quadro GP100 vs. [0] Quadro GV100     <- device 0 section, both GPUs' rows
    ...
    ## [1] Quadro GP100                           <- device 1 section, the same rows again
    ...
    - Total Matches: 176                          <- 88 comparable states
    

    --plot also gets 176 bars instead of 88. With --ignore-devices, device 0 is reported as [1] <ref GPU 1> vs. [0] <cmp GPU 0>.

  2. Crash when the last compare state has no reference state. A compare run that sweeps an extra axis value, such as Elements[pow2]=[16,20,24] against a reference that has only [16,20], is a partial comparison (Allow partial comparison in nvbench_compare.py #295). It fails with:

      File "python/scripts/nvbench_compare.py", line 562, in compare_benches
        ref_device = find_device_by_id(ref_state["device"], all_ref_devices)
    TypeError: 'NoneType' object is not subscriptable
    

Root cause

The loop for cmp_device_id in cmp_device_ids: walks every state without checking cmp_state["device"]. Afterwards, the reference device comes from ref_state, which is whatever the last loop iteration left behind. That value can be another device's state, or None when the last compare state was skipped as unmatched. The per-device loop has never filtered states by device. The ref_state["device"] lookup was added with --ignore-devices (#297).

Fix

  • Skip states that don't belong to the current device. This is the grouping the C++ markdown printer, nvbench_compare_robust.py and nvbench_json_summary.py already use.
  • Record the reference device from the states that actually produced rows.

The change is 4 lines in compare_benches(). Output for single-GPU runs is the same, except that the crash above is gone.

Verification

  • Two regression tests added to python/test/test_nvbench_compare.py: per-device sections, and a trailing compare state with no reference state. Both fail on main, one with the wrong header and one with the TypeError, and both pass with this change. They use a fake tabulate, like the robust-compare tests, so they also pass on Python 3.10 without the compare extra installed.
  • pytest python/test/test_nvbench_compare.py python/test/test_nvbench_tooling_deps.py python/test/test_nvbench_compare_robust.py: 181 passed.
  • On test_ref.json/test_cmp.json, the output now has ## [0] Quadro GV100 and ## [1] Quadro GP100 sections with Total Matches: 88. The multiset of comparison rows matches one section of the old output. --plot gets 88 entries. --ignore-devices labels each section [n] ref vs. [n] cmp.
  • ruff check, ruff format --check (v0.15.20, the pre-commit pin) and codespell pass.
  • The script only reads JSON, so none of this needs a GPU. I did not run the CUDA wheel CI jobs.

🤖 Generated with Claude Code

compare_benches() loops over the compare benchmark's devices, but the
inner loop walked every state of every device and then looked up the
reference device through the `ref_state` variable left over from the
last loop iteration. As a result:

- With results from more than one GPU, every "## [id] name" section
  listed the states of all GPUs, sections were labelled with the device
  of whichever state happened to be processed last (the bundled
  test_ref.json/test_cmp.json print "## [1] Quadro GP100 vs. [0] Quadro
  GV100" for device 0), and the summary counted each state once per
  device.
- When the last compare state has no reference counterpart (for example
  the compare run sweeps an extra axis value), `ref_state` is None and
  the script crashed with "TypeError: 'NoneType' object is not
  subscriptable".

Only compare the states that belong to the current device, as
nvbench_compare_robust.py and nvbench_json_summary.py already do, and
take the reference device from the states that produced rows.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@copy-pr-bot

copy-pr-bot Bot commented Oct 22, 2026

Copy link
Copy Markdown

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

This branch has not been deployed

No deployments
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.

1 participant