镜像站点 · 本页由第三方 GitHub 只读镜像提供,非 GitHub 官方站点,不接受任何登录或凭据输入。前往 github.com
Skip to content

Fix ROUGE loss adaptation for scalar Evaluate results - #1633

Merged
Li Jiang (thinkall) merged 6 commits into
microsoft:mainfrom
student-shivamm:codex/rouge-evaluate-scores
Oct 19, 2026
Merged

Li Jiang (thinkall) merged 6 commits into
microsoft:mainfrom
student-shivamm:codex/rouge-evaluate-scores

Conversation

@student-shivamm

@student-shivamm student-shivamm commented Oct 13, 2026 •

Copy link
Copy Markdown
Contributor

Why are these changes needed?

FLAML's ROUGE adapter assumes each returned score is a legacy AggregateScore and reads .mid.fmeasure. Hugging Face Evaluate's ROUGE implementation instead returns a scalar F-measure. This raises AttributeError during rouge1/rouge2 evaluation, including for identical prediction/reference summaries.

Accept the scalar result directly while retaining extraction of the legacy aggregate's F-measure. The existing 1 - score loss 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

  • Identical 61-case focused suite on upstream base 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.
  • Real official Hugging Face ROUGE script returns numpy.float64(1.0) for identical summaries. The adapter raises on the base and returns loss 0.0 with 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.
  • Direct checks using the repository-pinned Black 23.3.0, Ruff 0.0.261, pyupgrade 2.31.1 and the eight applicable pre-commit-hooks 4.4.0 entry points pass on the two changed Python files. The complete pre-commit runner stalled launching its Windows console scripts, including serial mode; its success is not claimed.
  • Windows x64, Python 3.12.14, Evaluate 0.4.6, datasets 3.6.0, rouge-score 0.1.2, NumPy 1.26.4. One existing NumPy reload warning occurs on both versions. Complete optional-dependency suite, transformer training and documentation build were not run.

Checks

  • Complete pre-commit runner passed (blocked locally as described above).
  • No documentation/API change required for restoring existing metric behavior.
  • Regression tests added.
  • Upstream automated checks passed (awaiting upstream CI).

@student-shivamm

Copy link
Copy Markdown
Contributor Author

@microsoft-github-policy-service agree

Copilot AI 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.

🟡 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

Comment thread test/automl/test_rouge_metric.py Resolved

@thinkall Li Jiang (thinkall) 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.

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

Comment thread test/automl/test_rouge_metric.py Resolved

Copilot AI 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.

🟢 Approval recommended

The localized fix preserves existing behavior and has focused regression coverage, with no identified blocking issues.

No open issues 🎉

1 resolved since last review

Review effort: Balanced

@thinkall Li Jiang (thinkall) 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.

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

@thinkall
Li Jiang (thinkall) dismissed their stale review October 17, 2026 12:13

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

Copilot AI 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.

🟢 Approval recommended

The localized compatibility fix preserves existing behavior and has focused regression coverage, with no identified blocking issues.

No open issues 🎉

Review effort: Balanced

Copilot AI 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.

🟢 Approval recommended

The narrow compatibility fix preserves existing behavior and has focused regression coverage with no identified blocking issues.

No open issues 🎉

Review effort: Balanced

@thinkall Li Jiang (thinkall) 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.

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

@thinkall
Li Jiang (thinkall) added this pull request to the merge queue Oct 19, 2026
Merged via the queue into microsoft:main with commit 12b0f15 Oct 19, 2026
16 checks passed
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