Skip to content

Fix NCU CSV NaN parser crash on empty unit cells#134

Merged
Laurawly merged 2 commits into
mainfrom
jiannanWang/fix-ncu-csv-nan-parser
May 14, 2026
Merged

Fix NCU CSV NaN parser crash on empty unit cells#134
Laurawly merged 2 commits into
mainfrom
jiannanWang/fix-ncu-csv-nan-parser

Conversation

@jiannanWang

Copy link
Copy Markdown
Contributor

The NCU cvs report contains a header row including
"ID","Process ID","Process Name",…,"Kernel Name",…,"Block Size","Grid Size",…,"sm__cycles_active.avg",…
and a unit row
"","","","","","","","","","",…,"cycle","%","block","block",…

The units row is the one we're parsing. It contains a unit string for every metric column (cycle, %, block, register, …) but empty strings for the identifier columns (ID, Kernel Name, Block Size, Grid Size) — because IDs and names don't have units.

When pandas parses those "" cells, it loads them as numpy.float64(nan). Later tok in nan raises TypeError: argument of type 'float' is not iterable.

This PR fix the typeerror by making the detection NaN-safe.

The unit-row detection in ``load_ncu_metrics`` reads the first CSV row,
lowercases each cell, and checks whether any cell contains a
unit-marker substring. ``Series.str.lower()`` propagates ``pd.NA`` /
``NaN`` cells through as float NaN, and the subsequent
``any(tok in x for tok in unit_tokens)`` then raises::

    TypeError: argument of type 'float' is not iterable

This surfaces on real NCU outputs (e.g. profiles from the
``gemma3_swiglu`` benchmark in SOL-ExecBench) and aborts the entire
profiling step, which then propagates up to the worker as
``Profiling failed`` and the round produces 0 successful kernels.

Fix: chain ``.fillna("")`` after ``.str.lower()`` so NaN cells become
empty strings before the substring check. The matching logic itself is
unchanged — empty strings legitimately don't contain any of the unit
tokens.

Test plan:
- ``pytest tests/`` (existing suite passes)
- Reproduces no longer: running the optimizer against
  ``ka-review-gate-runs/gemma3_swiglu`` previously hit the TypeError on
  every worker's first NCU profile; with the patch the round produces
  successful kernels.
@meta-cla meta-cla Bot added the CLA Signed This label is managed by the Meta Open Source bot. label May 8, 2026

@kaiming-cheng kaiming-cheng 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.

LGTM!

@Laurawly Laurawly merged commit 54b1331 into main May 14, 2026
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA Signed This label is managed by the Meta Open Source bot.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants