Skip to content

[TAO-2319][Bugfix] Add class_name column to kpi_calc.csv output - #31

Merged
arihantj-nv merged 4 commits into
mainfrom
fix/kpi-analyze-class-name
Aug 20, 2026
Merged

[TAO-2319][Bugfix] Add class_name column to kpi_calc.csv output#31
arihantj-nv merged 4 commits into
mainfrom
fix/kpi-analyze-class-name

Conversation

@arihantj-nv

Copy link
Copy Markdown
Contributor

Summary

  • kpi_analyze.py built final_kpi_df with class_name (line 88) but the to_csv column list at line 112 omitted it
  • kpi_calc.csv had one unlabeled row per class — callers had to rely on stdout PrettyTable order to identify rows, breaking deft_verify.py KPI parsing
  • Fix: add "class_name" to the column list (one-liner)
  • 3 regression tests verify the column round-trips correctly and document the pre-fix behaviour

Test plan

  • test_class_name_in_csv_columns — class_name present in saved CSV
  • test_class_name_values_preserved — values round-trip correctly
  • test_old_column_list_lacks_class_name — documents the regression

🤖 Generated with Claude Code

arihantj-nv and others added 2 commits August 11, 2026 06:15
kpi_analyze.py built final_kpi_df with class_name (line 88) but the
to_csv column list at line 112 omitted it. kpi_calc.csv had one row per
class with no label — callers had to rely on stdout PrettyTable order to
identify rows, which is fragile and broke deft_verify.py KPI parsing.

Signed-off-by: Arihant Jain <313131360+arihantj-nv@users.noreply.github.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Guards the fix: verifies class_name appears in the saved CSV and that
values round-trip correctly. A third test documents the pre-fix column
list to make the regression explicit.

Signed-off-by: Arihant Jain <313131360+arihantj-nv@users.noreply.github.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@arihantj-nv
arihantj-nv requested a review from a team as a code owner August 11, 2026 13:16
@arihantj-nv

Copy link
Copy Markdown
Contributor Author

/build

@tao-pr-bot

tao-pr-bot Bot commented Aug 11, 2026

Copy link
Copy Markdown

For security reasons, CI does not run automatically on NVIDIA's runners — it must be triggered per commit.

  • Comment /build to run CI on your latest commit, and make sure blossom-ci is green before merging.
  • Re-run /build after each new push — CI is pinned to the head commit, so a stale run won't count.

Tip

Should this fix also ship in a release? Add a release/X.Y.Z label (matching the target release branch) and tao-cherry-pick-bot will backport it after merge.

Important

Before merging: every file needs a license header and every commit must be DCO signed-off — see the 📖 Contribution guide for details.

…pts)

The update-readme hook regenerated the README using untracked local files
(example_per_image_analysis.py, kpi_analyze_per_image.py) that are not in
the repo. CI doesn't have those files so the hook produced a different
README and failed. Align committed README with what CI generates.

Signed-off-by: Arihant Jain <313131360+arihantj-nv@users.noreply.github.com>
@arihantj-nv

Copy link
Copy Markdown
Contributor Author

/build

@ddevendran-nvidia ddevendran-nvidia 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.

Generally looks good, but would double-check details of Claude's work

Comment thread tests/test_kpi_analyze_regression.py Outdated
Comment thread tests/test_kpi_analyze_regression.py Outdated
@arihantj-nv

Copy link
Copy Markdown
Contributor Author

/build

The previous test restated the to_csv column list inside the test body, so it
passed whether or not the fix was in place -- it exercised pandas, not
kpi_analyze. Review flagged test_old_column_list_lacks_class_name as
unnecessary; the same objection applies to the other two.

Run analyze() end to end against a small KITTI fixture and assert on the
kpi_calc.csv it actually writes. Nothing in the KPI path opens an image, so the
fixture is two label files and a class mapping. Verified to fail with the fix
at kpi_analyze.py:112 reverted.

Also drops the io/StringIO indirection the review asked about -- the CSV is
written to a real results_dir now, so there is nothing to explain.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Signed-off-by: Arihant Jain <313131360+arihantj-nv@users.noreply.github.com>
@arihantj-nv
arihantj-nv force-pushed the fix/kpi-analyze-class-name branch from 41139cc to 45efa90 Compare August 19, 2026 22:39
@arihantj-nv

Copy link
Copy Markdown
Contributor Author

/build

@svc-bcs-agent

Copy link
Copy Markdown

Hi @arihantj-nv 👋💛

🤖✨ AIRE (NVIDIA's automated CI agent) here! ✨🤖

🔍🚨 I detected that the CI pipeline on your PR #31 has failed — but no worries, I'm already on it! 💪🔧

🛠️🔬 I'm automatically diagnosing the root cause right now and preparing a fix for you. ⏳✅

🔔💬 I'll ping you the moment the fix is ready to review — nothing you need to do for now. Sit back and relax! 🌿😌

💙🙏 Thanks for your patience — AIRE's got your back! 🚀🌟

@arihantj-nv
arihantj-nv merged commit 92c37d1 into main Aug 20, 2026
5 checks passed
@arihantj-nv
arihantj-nv deleted the fix/kpi-analyze-class-name branch August 20, 2026 16:11
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.

3 participants