Skip to content

fix(optimizer): robust metric resolution in slow-update longitudinal comparison - #257

Open
RohithPariki wants to merge 1 commit into
microsoft:mainfrom
RohithPariki:auto-fix-slow-update-robustness
Open

fix(optimizer): robust metric resolution in slow-update longitudinal comparison#257
RohithPariki wants to merge 1 commit into
microsoft:mainfrom
RohithPariki:auto-fix-slow-update-robustness

Conversation

@RohithPariki

Copy link
Copy Markdown
Contributor

Problem

In skillopt.optimizer.slow_update (build_comparison_pairs):
Longitudinal comparison between adjacent epoch rollouts categorized per-sample outcomes (improved, regressed, persistent_fail, stable_success) strictly using bool(prev.get("hard", 0)) and bool(curr.get("hard", 0)).

When running on benchmark environments that evaluate using score, exact_match, or floating-point soft thresholds without an explicit hard key in the result dictionary:

  • Successful rollouts were treated as failures (bool(0) == False), misclassifying genuine improvements into persistent_fail and regressions into stable_success.
  • Soft metric summaries defaulted to 0.0 even when the task succeeded under primary evaluation metrics.

Root Cause

Evaluation metric representations across benchmark adapters can vary (hard, score, exact_match, soft). build_comparison_pairs assumed an explicit binary integer hard key.

Solution

  • Introduced _is_result_success() in skillopt/optimizer/slow_update.py to evaluate outcome success across hard, score, exact_match, and soft metrics with robust type casting.
  • Updated build_comparison_pairs to use _is_result_success() for accurate categorization.
  • Added comprehensive unit tests in tests/test_slow_update_robustness.py covering metric categorization, field lifecycle, and serialization.

Testing

  • Ran unit tests: pytest tests/test_slow_update_robustness.py (4/4 passed).
  • Ran linter: ruff check (clean).

Risk

Low. Improves classification accuracy for non-binary and custom benchmark results while preserving exact behavior for existing hard-keyed results.

@RohithPariki
RohithPariki marked this pull request as ready for review August 26, 2026 01:35
@Yif-Yang

Copy link
Copy Markdown
Contributor

Thanks for trying to support additional metric shapes. The current fallback is too permissive for the existing rollout contract, which requires adapters to return hard and soft. Guessing success from generic fields with global thresholds can hide malformed adapters and change benchmark semantics; for example, a string hard="false" is truthy and becomes success, while synthesized soft=0/1 discards the original score. Please keep normalization in each benchmark adapter, or introduce an explicit configured metric resolver with benchmark-defined semantics. Invalid, string, non-finite, or out-of-range values should fail closed rather than be guessed.

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.

2 participants