Conversation
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
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
In the ownership-based near-end check for scored SGFs, all three counters and
boardAreaare integers. As a result,count / boardAreatruncates before comparison with0.10 + 6.0 / boardArea. For example, on a 19x19 board the ownership gate accepts 100 unsettled points because100 / 361is 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:
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 existingNoValueScoredNotNearEndingpath retain their behavior. A failed near-end check does not unconditionally discard the entire SGF.Validation
90126b1a0afa005927f759c2b2334351b30e53f0passed Eigen Releaseruntestsandrunoutputtests.runtestsplus fiverunoutputtestsrepetitions.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
masterindependently.The patch changes only
cpp/command/writetrainingdata.cpp,cpp/command/runtests.cpp, andcpp/tests/tests.h, based ond91ea855110dae533f0aada947b2b7d78cc8a4e1. 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.