Comfy-Org/comfyui-benchmark

Fork 0

mirror of https://github.com/Comfy-Org/comfyui-benchmark.git synced 2026-11-04 23:47:33 +00:00

[PR #4] Fix nvidia-smi calls when --cuda-device is not set #3

New issue

Open

opened 2026-10-26 22:11:23 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-10-26 22:11:23 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/Comfy-Org/comfyui-benchmark/pull/4
Author: @gerket
Created: 9/3/2026
Status: 🔄 Open

Base: main ← Head: fix-empty-smi-id-argument

📝 Commits (1)

776a83e Fix nvidia-smi calls when --cuda-device is not set

📊 Changes

1 file changed (+5 additions, -3 deletions)

View changed files

📝 __init__.py (+5 -3)

📄 Description

Problem

smi_id is built as a string and appended to a list-form argv:

smi_id = "" if cuda_device_arg is None else f"--id={cuda_device_arg}"
_nvidia_smi_query_list = ["nvidia-smi", "--query-gpu=...", "--format=csv,noheader,nounits", smi_id]

With no --cuda-device, smi_id is "" — and since subprocess passes a list through verbatim, the child receives a real, empty argument rather than nothing:

ERROR: Option  is not recognized. Please run 'nvidia-smi -h' for help.

(Note the double space — the option name is the empty string.) Exit code 2 raises CalledProcessError, so call_nvidia_smi returns None for both probes in BenchmarkExtension.get_node_list, and ENABLE_NVIDIA_SMI_DATA is never set.

The habit comes from shells, where an empty unquoted variable does vanish through word splitting — but "$var" quoted is an empty argument, the identical failure. A list has no word-splitting step at all, which is the point of the list form, so the conditional has to produce the list rather than a member of it.

Impact

--cuda-device defaults to None, so this affects every run that doesn't explicitly pass it. Those benchmarks record:

info_nvidia_smi_query: null — no GPU name, driver version or vBIOS
initial_nvidia_smi_query: null — no starting VRAM/util/power/PCIe reading
nvidia_smi_data: [] — the sampling thread never starts, since it's gated on ENABLE_NVIDIA_SMI_DATA

It fails silently. A failed probe is indistinguishable from the feature being switched off deliberately, so the benchmark JSON looks normal apart from the null fields. The error is captured in nvidia_smi_error, which is what made it findable — that field held the message above on every run.

Fix

Make smi_id a list and splat it, so "absent" is genuinely absent.

Verification

nvidia-smi 616.56, RTX 4090, Windows, ComfyUI 0.33.4 — both branches, before and after:

	`cuda_device`	argc	result
before	`None`	4	exit 2
before	`0`	4	ok
after	`None`	3	ok
after	`0`	4	ok

The working branch is unchanged; only the broken one moves.

Confirmed end to end on a live instance: after passing --cuda-device 0 as a workaround, nvidia_smi_error cleared and both query fields populated (NVIDIA GeForce RTX 4090, 1, 616.56, Enabled, 95.02.3c.40.d1, …). This patch removes the need for that workaround.

_{🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.}

## 📋 Pull Request Information **Original PR:** https://github.com/Comfy-Org/comfyui-benchmark/pull/4 **Author:** [@gerket](https://github.com/gerket) **Created:** 9/3/2026 **Status:** 🔄 Open **Base:** `main` ← **Head:** `fix-empty-smi-id-argument` --- ### 📝 Commits (1) - [`776a83e`](https://github.com/Comfy-Org/comfyui-benchmark/commit/776a83ede75904680fbc6e5c0723feb0ee60d8ca) Fix nvidia-smi calls when --cuda-device is not set ### 📊 Changes **1 file changed** (+5 additions, -3 deletions) <details> <summary>View changed files</summary> 📝 `__init__.py` (+5 -3) </details> ### 📄 Description ## Problem `smi_id` is built as a string and appended to a list-form argv: ```python smi_id = "" if cuda_device_arg is None else f"--id={cuda_device_arg}" _nvidia_smi_query_list = ["nvidia-smi", "--query-gpu=...", "--format=csv,noheader,nounits", smi_id] ``` With no `--cuda-device`, `smi_id` is `""` — and since `subprocess` passes a list through verbatim, the child receives a real, empty argument rather than nothing: ``` ERROR: Option is not recognized. Please run 'nvidia-smi -h' for help. ``` (Note the double space — the option name is the empty string.) Exit code 2 raises `CalledProcessError`, so `call_nvidia_smi` returns `None` for both probes in `BenchmarkExtension.get_node_list`, and `ENABLE_NVIDIA_SMI_DATA` is never set. The habit comes from shells, where an empty *unquoted* variable does vanish through word splitting — but `"$var"` quoted is an empty argument, the identical failure. A list has no word-splitting step at all, which is the point of the list form, so the conditional has to produce the list rather than a member of it. ## Impact `--cuda-device` defaults to `None`, so this affects every run that doesn't explicitly pass it. Those benchmarks record: - `info_nvidia_smi_query: null` — no GPU name, driver version or vBIOS - `initial_nvidia_smi_query: null` — no starting VRAM/util/power/PCIe reading - `nvidia_smi_data: []` — the sampling thread never starts, since it's gated on `ENABLE_NVIDIA_SMI_DATA` It fails silently. A failed probe is indistinguishable from the feature being switched off deliberately, so the benchmark JSON looks normal apart from the null fields. The error *is* captured in `nvidia_smi_error`, which is what made it findable — that field held the message above on every run. ## Fix Make `smi_id` a list and splat it, so "absent" is genuinely absent. ## Verification nvidia-smi 616.56, RTX 4090, Windows, ComfyUI 0.33.4 — both branches, before and after: | | `cuda_device` | argc | result | |---|---|---|---| | before | `None` | 4 | **exit 2** | | before | `0` | 4 | ok | | after | `None` | 3 | ok | | after | `0` | 4 | ok | The working branch is unchanged; only the broken one moves. Confirmed end to end on a live instance: after passing `--cuda-device 0` as a workaround, `nvidia_smi_error` cleared and both query fields populated (`NVIDIA GeForce RTX 4090, 1, 616.56, Enabled, 95.02.3c.40.d1, …`). This patch removes the need for that workaround. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

bsakaguchi added the

pull-request

label

2026-10-26 22:11:23 +00:00

No labels

pull-request

No milestone

No project

No assignees

1 participant

Notifications

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference

Comfy-Org/comfyui-benchmark#3

No description provided.

Rows
Columns