Skip to content

Fix integer division and boundary errors in scored-SGF near-end detection - #1259

Open
22nsuk wants to merge 1 commit into
lightvector:masterfrom
22nsuk:codex/upstream-near-end-exact
Open

22nsuk wants to merge 1 commit into
lightvector:masterfrom
22nsuk:codex/upstream-near-end-exact

Conversation

@22nsuk

@22nsuk 22nsuk commented Oct 5, 2026

Copy link
Copy Markdown

In the ownership-based near-end check for scored SGFs, all three counters and boardArea are integers. As a result, count / boardArea truncates before comparison with 0.10 + 6.0 / boardArea. For example, on a 19x19 board the ownership gate accepts 100 unsettled points because 100 / 361 is zero, although the intended strict threshold permits at most 42. The preceding score/win-loss checks still apply independently.

This patch evaluates the intended condition exactly for positive board areas:

10LL * count < (int64_t)boardArea + 60

Widening before multiplication and addition avoids integer truncation and floating-point boundary drift. A double-only fix can still accept the exact boundary on some rectangular boards; for a 10x12 board, 18 unsettled points must be rejected.

The three ownership comparisons are factored into a file-local isNearEndByOwnership() helper shared by the production path and the native regression tests. Ownership counting, score/win-loss checks, subsequent cleanup and target handling, and the existing NoValueScoredNotNearEnding path retain their behavior. A failed near-end check does not unconditionally discard the entire SGF.

Validation

  • The native test sweeps each counter independently, plus all three counters together, from zero through the board area on ten board shapes: 9x9, 10x10, 13x13, 19x19, and six rectangles. It makes 6,524 assertions using explicitly specified expected boundary counts.
  • Boundary regression validation confirmed that the earlier double comparison fails the rectangular boundary cases. The run also records a separate upstream visit-cap output-test failure and is not presented as an overall passing run.
  • Standalone candidate build: the exact upstream-only candidate 90126b1a0afa005927f759c2b2334351b30e53f0 passed Eigen Release runtests and runoutputtests.
  • Combined validation: this patch together with the terminal-leaf test correction in Fix visit-cap test invariants for terminal leaves #1258 was rebuilt and passed runtests plus five runoutputtests repetitions.

Related test correction

Standalone repeated output testing exposed a pre-existing visit-cap test invariant that can reject terminal leaves. A deterministic fixture reproduces that failure without this arithmetic change or any production engine changes. #1258 addresses it separately and would preferably merge first. This branch does not include that test correction and targets master independently.

The patch changes only cpp/command/writetrainingdata.cpp, cpp/command/runtests.cpp, and cpp/tests/tests.h, based on d91ea855110dae533f0aada947b2b7d78cc8a4e1. Validation covers the production predicate and native engine tests; it does not include GPU inference or an end-to-end SGF-to-NPZ conversion/dataset impact study.

Replace truncated ownership fractions with exact widened integer comparisons. Exercise the production predicate on square and rectangular boards through runtests. Preserve the lead/win-loss checks and downstream target handling.

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