[Bugfix] Fix np.float64 leaking into the KV transfer metrics log by NickLucche · Pull Request #57068 · vllm-project/vllm · GitHub
Skip to content

[Bugfix] Fix np.float64 leaking into the KV transfer metrics log - #57068

Merged
NickLucche merged 4 commits into
vllm-project:mainfrom
NickLucche:fix-kv-metrics-np-float64-log
Sep 16, 2026
Merged

NickLucche merged 4 commits into
vllm-project:mainfrom
NickLucche:fix-kv-metrics-np-float64-log

Conversation

@NickLucche

Copy link
Copy Markdown
Member

The KV transfer metrics log line currently shows raw numpy scalar reprs when the stats come from a connector inside a MultiConnector:

(APIServer pid=3547631) INFO 09-15 19:44:52 [metrics.py:107] KV Transfer metrics: NixlPullConnector={'Num successful transfers': 8, 'Avg xfer time (ms)': np.float64(2.338), 'P90 xfer time (ms)': 2.772, 'Avg post time (ms)': np.float64(1.111), 'P90 post time (ms)': 1.88, 'Avg MB per transfer': np.float64(25.441), 'Throughput (MB/s)': np.float64(10883.285), 'Avg number of descriptors': np.float64(133.5)}

The single-connector path was fine because the log line formats each value with an f-string (str()), which renders np.float64 as a plain number. With a MultiConnector, reduce() returns a nested dict and the whole dict gets rendered with str(), which formats every value with repr() — and numpy 2 renders repr(np.float64(2.338)) as np.float64(2.338).

The fix is to have the nixl/mooncake/hf3fs reduce() implementations return plain Python scalars via .item(), which is what their dict[str, int | float] annotation already promised. After the fix the same line reads:

KV Transfer metrics: NixlPullConnector={'Num successful transfers': 8, 'Avg xfer time (ms)': 2.338, 'P90 xfer time (ms)': 2.772, 'Avg post time (ms)': 1.111, 'P90 post time (ms)': 1.88, 'Avg MB per transfer': 25.441, 'Throughput (MB/s)': 10883.285, 'Avg number of descriptors': 133.5}

Assisted-by: Mistral Vibe

reduce() in the nixl/mooncake/hf3fs connector stats returned numpy
scalars (np.float64), violating the declared dict[str, int | float]
contract. The single-connector log path hid this because f-strings use
str(), but with a MultiConnector the reduced stats form a nested dict
whose str() renders every value with repr(), so the CLI log showed
np.float64(2.338) instead of 2.338.

Convert the reduced values to plain Python scalars via .item().

Generated by Mistral Vibe.
Co-Authored-By: Mistral Vibe <vibe@mistral.ai>

Signed-off-by: NickLucche <nicolo.lucchesi@mistral.ai>

@claude claude 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.

Claude Code Review

This pull request is from a fork — automated review is disabled. A repository maintainer can comment @claude review to run a one-time review.

@mergify mergify Bot added bug Something isn't working kv-connector labels Sep 15, 2026
@NickLucche

Copy link
Copy Markdown
Member Author

@github-actions

Copy link
Copy Markdown

✅ Triggered Buildkite CI #89169 for commit 62ba39162a63.

@chaunceyjiang chaunceyjiang added the ready ONLY add when PR is ready to merge/full CI is needed label Sep 16, 2026
@chaunceyjiang

Copy link
Copy Markdown
Contributor

/ci run

@chaunceyjiang chaunceyjiang left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks~

@github-actions

Copy link
Copy Markdown

✅ CI is already running for this commit: https://buildkite.com/vllm/ci/builds/89169

@NickLucche

Copy link
Copy Markdown
Member Author

/ci retry

@github-actions

Copy link
Copy Markdown

✅ The previous CI build is still running: https://buildkite.com/vllm/ci/builds/89169

@NickLucche

Copy link
Copy Markdown
Member Author

/ci run

@github-actions

Copy link
Copy Markdown

@NickLucche
NickLucche enabled auto-merge (squash) September 16, 2026 16:19
@NickLucche
NickLucche merged commit fc8132a into vllm-project:main Sep 16, 2026
60 checks passed
michael-gpu pushed a commit to practical-inference/vllm-ampere-extended that referenced this pull request Oct 20, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working kv-connector ready ONLY add when PR is ready to merge/full CI is needed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants