Skip to content

Count invalid event-verification answers as abstentions - #350

Open
sylvesterkaczmarek wants to merge 2 commits into
NVIDIA:mainfrom
sylvesterkaczmarek:fix/event-verification-abstentions-20261101
Open

sylvesterkaczmarek wants to merge 2 commits into
NVIDIA:mainfrom
sylvesterkaczmarek:fix/event-verification-abstentions-20261101

Conversation

@sylvesterkaczmarek

@sylvesterkaczmarek sylvesterkaczmarek commented Nov 1, 2026 •

Copy link
Copy Markdown

Resubmits NVIDIA/cosmos#365 at the new Cosmos Framework evaluation path, as requested by @yaoxu-crypto after the evaluation suites moved under .agents/skills/. The original change addresses NVIDIA/cosmos#364.

Correction

  • Count unparseable or missing event-verification answers as abstentions, including a false negative for the ground-truth class.
  • Exclude the synthetic abstention class from macro-F1, preserve per-class and weighted metrics and all-row accuracy, and omit the misleading in-label-only micro average on partly invalid runs.
  • Keep Valid Predictions as parse coverage and skip NaN ground truth even when it came from a matched annotation row.
  • Keep empty/unscored input behavior unchanged.

Metric comparability
The abstention-aware macro_f1 is not directly comparable with legacy reports that discarded invalid responses. The docstring no longer claims the published VANTAGE Table 2 numbers were computed using this policy; that historical methodology has not been verified.

Verification

  • All 16 focused regression/control tests passed against the migrated code using real pandas, scikit-learn, evaluator logic, and score-reporting file I/O on macOS arm64/Python 3.12. The additional regressions pin the report schema under partial abstentions and NaN matched ground truth.
  • python -m py_compile and git diff --check passed.

Focused command from .agents/skills/evaluation/cosmos3/reasoner/vlmevalkit with the CPU dependencies installed:

python -m pytest -c /dev/null --confcutdir=tests -q tests/test_event_verification.py

The same focused invocation is documented in tests/README.md. Using -c /dev/null keeps this vendored evaluator unit run independent of the framework root’s broader pytest configuration. These tests are not claimed to run in the root CI workflow. No video downloading, model inference, GPU workloads, or full benchmark runs were performed. No runtime dependencies were changed.

Original PR: NVIDIA/cosmos#365. DCO sign-off is included. Thanks for the migration instructions.

Signed-off-by: Sylvester Kaczmarek <16242628+sylvesterkaczmarek@users.noreply.github.com>

@pengcuo pengcuo left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Review notes. The logic is correct: I traced the parametrized cases (e.g. truth yes,no,no / pred yes,?,? -> yes F1 1.0, no F1 0.0 -> 0.5), and labels= without the sentinel does count those rows as false negatives.

  1. This changes the headline score, but the docstring still says it matches the paper. evaluate() still says macro_f1 "matches VANTAGE paper Table 2". Before, unparseable answers were dropped. Now they lower recall, so the same predictions score lower whenever any answer fails to parse. Results from before and after this PR are not comparable. Either confirm that Table 2 counts parse failures as abstentions and say so in the docstring, or keep the old number under its own key. Otherwise people will compare new runs with published numbers.

  2. The output columns now depend on the data. Because labels omits the sentinel, sklearn emits micro avg instead of accuracy only when some answer is invalid. The code then overwrites accuracy, so those runs get both accuracy and micro avg--*, and fully valid runs get only accuracy. The micro avg values also ignore the abstained rows (precision/recall over in-label predictions only), which is misleading next to accuracy. Drop micro avg from the report when the sentinel is present, or compute precision/recall/F1 from the same all-rows basis. The tests only compare against classification_report for fully valid inputs, so this is not covered.

  3. Low: a matched ground-truth row with a NaN answer becomes the class 'nan'. if pd.isna(gt): continue only guards the gt_row is None branch. A NaN gt_row['answer'] becomes str(gt) = 'nan', and labels now includes nan in the macro average. This already happened for valid predictions, but with abstentions kept it now also applies to unparseable ones. A one-line isna check after the lookup fixes both cases.

  4. Low: the tests need -c /dev/null. The PR description says the focused run needs -c /dev/null --confcutdir=tests, so they are not collectable under the repo's pytest config. Please either make them run normally in CI or state where they run.

Signed-off-by: Sylvester Kaczmarek <16242628+sylvesterkaczmarek@users.noreply.github.com>
@sylvesterkaczmarek

Copy link
Copy Markdown
Author

Thanks, @pengcuo. I addressed the report consistency and documentation points in signed commit 83b3af8: the evaluator now explicitly warns that abstention-aware macro-F1 cannot be compared directly with legacy drop-invalid scores, removes sklearn’s misleading partial micro-average, and skips NaN ground truth after annotation lookup. Added two regressions and documented the isolated test command under tests/README.md; all 16 focused tests pass. Root CI integration and historical VANTAGE Table 2 methodology are not claimed as verified.

@yaoxu-crypto yaoxu-crypto added the Skill New skill or updating existing skill label Nov 3, 2026

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

Skill New skill or updating existing skill

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants