Skip to content

fix(kernelforge): reject failed allclose verdicts alongside SNR - #2333

Draft
Zhu-Jingwen wants to merge 1 commit into
AMD-AGI:mainfrom
Zhu-Jingwen:fix/forge-allclose-verdict
Draft

Zhu-Jingwen wants to merge 1 commit into
AMD-AGI:mainfrom
Zhu-Jingwen:fix/forge-allclose-verdict

Conversation

@Zhu-Jingwen

Copy link
Copy Markdown

A zero-exit driver printing SNR: 60.0 dB and allclose: False passes a 30 dB threshold because the SNR branch ignores the failed verdict. Reading only the first allclose match also lets a later failed case or state check hide behind an earlier success.

Aggregate every recognized allclose verdict and require no failed check alongside the existing SNR threshold. Preserve nonzero-exit rejection and existing SNR-only/allclose-only behavior.

allclose_matches = re.findall(r"allclose:\s*(true|false)", full_output.casefold())
allclose = "false" not in allclose_matches if allclose_matches else None
passed = snr_db >= snr_threshold and allclose is not False

Observable effect: any emitted failed allclose verdict rejects the candidate even when SNR is high. This intentionally tightens acceptance policy while keeping numerical threshold values unchanged. Emitted allclose remains a check verdict; optional diagnostics must use a different label.

Validation:

  • 276 focused CPU evaluation cases passed.
  • 60 dB/False and both True/False orders fail; 30 dB/True passes; 29.9 dB/True fails; exit 3 remains a driver error.
  • Existing SNR-only, allclose-only, missing-metric and validation-pipeline cases remain covered.
  • Ruff 0.16.2 lint/format and the code-metrics gate passed against the stated base. No metrics baseline was raised.

Single concern: correctness verdict aggregation. No CLI or schema change. This validates emitted checks; it does not enforce omitted checks or protect an edited oracle. No generated-candidate GPU qualification is claimed.

Lineage: preserves Huazeng Li's author credit from the originating fix. Jingwen Zhu adapted and verified the patch against the current source layout.

Base: d3df5b59f4712648d69cde2286dffac8040bc0d4.

Full local CPU regression (pytest -m "not targeted_build_e2e" -q -n 4): 40 failed, 26897 passed, 64 skipped, 28 warnings, 21 errors, 3 subtests passed in 603.36s (0:10:03). The 40 failed node IDs and 21 collection-error IDs exactly match unchanged main (40 failed, 26891 passed, 64 skipped, 28 warnings, 21 errors, 3 subtests passed in 607.33s (0:10:07)). This change introduces no additional failed or error node in this environment; this is not a green full-suite claim. The PR remains draft pending upstream CI and review.

Verified baseline regression control: 3 failed, 3 passed, 20 deselected in 0.36s. Those fixtures were run outside the patched worktree with an assertion that the imported production module came from unchanged upstream main. The corresponding fixed cases pass in the focused selection.

Aggregate every recognized allclose verdict and require no failed check alongside the existing SNR threshold. Preserve SNR-only, allclose-only and nonzero-exit behavior; cover contradictory metrics and both mixed-verdict orders.

Co-authored-by: Huazeng Li <Huazeng.Li@amd.com>

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

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant