Repository navigation
Fix ROUGE loss adaptation for scalar Evaluate results - #1633
Li Jiang (thinkall) merged 6 commits into
Conversation
|
@microsoft-github-policy-service agree |
There was a problem hiding this comment.
🟡 Changes recommended
New tests unconditionally import optional dependencies excluded from Windows ARM64 CI installations.
1 open issue
What changed in this PR
Restores ROUGE evaluation compatibility with scalar Evaluate results while preserving legacy aggregate support.
Changes:
- Accept scalar ROUGE scores while retaining legacy F-measure extraction.
- Add regression tests for score formats, loaders, loss direction, and errors.
| File | Description |
|---|---|
test/automl/test_rouge_metric.py |
Adds ROUGE compatibility and error-handling tests. |
flaml/automl/ml.py |
Supports scalar and legacy aggregate ROUGE scores. |
Review effort: Balanced
Li Jiang (thinkall)
left a comment
There was a problem hiding this comment.
Overall review of the complete PR: changes requested for the new tests' Windows ARM64 compatibility.
The production change correctly accepts scalar ROUGE values and preserves legacy AggregateScore extraction and loss direction. All 39 focused tests passed locally after restoring missing test dependencies in an isolated environment. That does not cover the repository's Windows ARM64 test-dependency contract: the new tests import datasets/evaluate even though those packages are explicitly excluded there. Please address the inline finding with module stubs or capability-aware test guards. The review-code harness and the current Balanced Copilot review identify this blocker; CI is also not fully green.
Posted by thinkall-agent-auto-reviewer
Li Jiang (thinkall)
left a comment
There was a problem hiding this comment.
Overall review of the complete updated PR: approved.
The ROUGE scalar/legacy adaptation preserves the loss direction, and the pytest-scoped module stubs remove the Windows ARM64 dependency mismatch from the new tests. All 39 focused cases pass locally with the real datasets/evaluate modules blocked; this checks the optional-dependency contract, not a native ARM64 machine. I verified the fix and resolved my earlier thread. The review-code Low harness is clean, all review threads are resolved, current-head CI checks have passed, and Copilot's current Balanced review explicitly recommends approval.
Posted by thinkall-agent-auto-reviewer
Withdrawing this approval: a final check of the workflow-run API found that the current-head build, docs and formatting runs still require approval. The green check rollup did not establish that those workflows had passed. The source fix remains verified and my resolved finding stays resolved, but PR approval must wait for successful completion of the actual CI runs.
Posted by thinkall-agent-auto-reviewer
Li Jiang (thinkall)
left a comment
There was a problem hiding this comment.
Overall review at 97d503859e50443e0f0b8f72e90d064c7861385d: re-reviewed the complete PR with review-code. The adapter accepts Python/NumPy scalar ROUGE results and legacy aggregate results, then preserves the existing maximization-to-loss conversion. Loader fallback, non-ROUGE loss direction, and public dependency/missing-metric errors remain unchanged. No remaining correctness issue found.
Validation: python -m pytest test\automl\test_rouge_metric.py -q --disable-warnings passed (39 passed), covering scalar/aggregate types, both mocked loader paths, forwarding, loss direction, and error contracts. A missing local rouge-score test dependency was restored only in an isolated environment before that run. The whole-PR Low harness was clean, and its provenance was audited and committed locally. All discussion threads are resolved, current-head Balanced Copilot explicitly recommends approval, check contexts pass, and the actual Build, docs, and Code formatting workflow runs completed successfully on this head.
Posted by thinkall-agent-auto-reviewer

Why are these changes needed?
FLAML's ROUGE adapter assumes each returned score is a legacy
AggregateScoreand reads.mid.fmeasure. Hugging Face Evaluate's ROUGE implementation instead returns a scalar F-measure. This raisesAttributeErrorduringrouge1/rouge2evaluation, including for identical prediction/reference summaries.Accept the scalar result directly while retaining extraction of the legacy aggregate's F-measure. The existing
1 - scoreloss direction and metric loading behavior remain unchanged. No new dependency or public API is introduced.Related issue number
No existing matching issue found in the public ROUGE issue/PR search on 2026-10-13. This is a standalone compatibility fix.
Validation
6466f0771fc435f826d689c53149f6a0d9621e51: 24 failed / 37 passed; candidate: 61 passed. The failures concern scalar ROUGE results. Tests also cover legacy aggregates, NumPy scalars, both loader paths, zero/partial/perfect scores, unchanged loss direction and dependency/metric errors. Existing metric and tuning checks are included.numpy.float64(1.0)for identical summaries. The adapter raises on the base and returns loss0.0with this change for both supported ROUGE variants. This check uses a downloaded official script and reuses the real metric instance through a patched loader; it is not an end-to-end Hub loading test. Default Hub loading failed separately in this environment.Checks