Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Correctly prevent recursive verification in nmp. #1610

Closed
wants to merge 2 commits into from

Conversation

vondele
Copy link
Member

@vondele vondele commented May 23, 2018

As discussed with @pb00068, the condition to prevent recursive verification was not completely correct. This patch corrects that condition, and adds an assert. In the current implementation, recursive verification needs to be avoided in order not to break the verification of the move closer to the root (i.e. to not set thisThread->nmp_min_ply to zero prematurely).

This patch is tested as a bug fix, based on and tested against PR #1609 .

passed STC:
http://tests.stockfishchess.org/tests/view/5b050f170ebc5914abc129c5
LLR: 2.95 (-2.94,2.94) [-3.00,1.00]
Total: 7875 W: 1669 L: 1523 D: 4683

passed LTC:
http://tests.stockfishchess.org/tests/view/5b0513970ebc5914abc129cd
LLR: 3.00 (-2.94,2.94) [-3.00,1.00]
Total: 24048 W: 3525 L: 3407 D: 17116

changing at high depth, bench: 4484288

define Color us and use this instead of pos.side_to_move() and nmp_odd. The latter allows to clarify the nmp verification criterion.

Tested for no regression:
LLR: 2.95 (-2.94,2.94) [-3.00,1.00]
Total: 76713 W: 15303 L: 15284 D: 46126
http://tests.stockfishchess.org/tests/view/5b046a0d0ebc5914abc12971

No functional change.
@vondele
Copy link
Member Author

vondele commented May 23, 2018

seems like appveyor CI is failing randomly again. Closing and reopening this PR to try and trigger retesting.

@vondele vondele closed this May 23, 2018
@vondele vondele reopened this May 23, 2018
@vondele vondele closed this May 23, 2018
@vondele vondele reopened this May 23, 2018
As discussed with @pb00068, the condition to prevent recursive verification was not completely correct. This patch corrects that condition, and adds an assert. In the current implementation, recursive verification needs to be avoided in order not to break the verification of the move closer to the root (i.e. to not set thisThread->nmp_min_ply to zero prematurely).

This patch is tested as a bug fix, based on and tested against PR official-stockfish#1609 .

passed STC:
http://tests.stockfishchess.org/tests/view/5b050f170ebc5914abc129c5
LLR: 2.95 (-2.94,2.94) [-3.00,1.00]
Total: 7875 W: 1669 L: 1523 D: 4683

passed LTC:
http://tests.stockfishchess.org/tests/view/5b0513970ebc5914abc129cd
LLR: 3.00 (-2.94,2.94) [-3.00,1.00]
Total: 24048 W: 3525 L: 3407 D: 17116

changing at high depth, bench: 4484288
@snicolet
Copy link
Member

Approved

@snicolet snicolet closed this in beaadd6 May 24, 2018
@snicolet
Copy link
Member

Merged via beaadd6, thanks!

goodkov pushed a commit to goodkov/Stockfish that referenced this pull request Jul 21, 2018
As discussed with @pb00068, the condition to prevent recursive verification
was not completely correct. This patch corrects that condition, and adds an
assert. In the current implementation, recursive verification needs to be
avoided in order not to break the verification of the move closer to the
root (i.e. to not set thisThread->nmp_min_ply to zero prematurely).

This patch is tested as a bug fix, based on and tested against PR official-stockfish#1609 .

passed STC:
http://tests.stockfishchess.org/tests/view/5b050f170ebc5914abc129c5
LLR: 2.95 (-2.94,2.94) [-3.00,1.00]
Total: 7875 W: 1669 L: 1523 D: 4683

passed LTC:
http://tests.stockfishchess.org/tests/view/5b0513970ebc5914abc129cd
LLR: 3.00 (-2.94,2.94) [-3.00,1.00]
Total: 24048 W: 3525 L: 3407 D: 17116

Bench changes at high depth.

Closes official-stockfish#1610
and    official-stockfish#1609

Bench: 4484288
CounterPly pushed a commit to CounterPly/Stockfish that referenced this pull request Jul 24, 2018
As discussed with @pb00068, the condition to prevent recursive verification
was not completely correct. This patch corrects that condition, and adds an
assert. In the current implementation, recursive verification needs to be
avoided in order not to break the verification of the move closer to the
root (i.e. to not set thisThread->nmp_min_ply to zero prematurely).

This patch is tested as a bug fix, based on and tested against PR official-stockfish#1609 .

passed STC:
http://tests.stockfishchess.org/tests/view/5b050f170ebc5914abc129c5
LLR: 2.95 (-2.94,2.94) [-3.00,1.00]
Total: 7875 W: 1669 L: 1523 D: 4683

passed LTC:
http://tests.stockfishchess.org/tests/view/5b0513970ebc5914abc129cd
LLR: 3.00 (-2.94,2.94) [-3.00,1.00]
Total: 24048 W: 3525 L: 3407 D: 17116

Bench changes at high depth.

Closes official-stockfish#1610
and    official-stockfish#1609

Bench: 4484288
CounterPly pushed a commit to CounterPly/Stockfish that referenced this pull request Jul 30, 2018
As discussed with @pb00068, the condition to prevent recursive verification
was not completely correct. This patch corrects that condition, and adds an
assert. In the current implementation, recursive verification needs to be
avoided in order not to break the verification of the move closer to the
root (i.e. to not set thisThread->nmp_min_ply to zero prematurely).

This patch is tested as a bug fix, based on and tested against PR official-stockfish#1609 .

passed STC:
http://tests.stockfishchess.org/tests/view/5b050f170ebc5914abc129c5
LLR: 2.95 (-2.94,2.94) [-3.00,1.00]
Total: 7875 W: 1669 L: 1523 D: 4683

passed LTC:
http://tests.stockfishchess.org/tests/view/5b0513970ebc5914abc129cd
LLR: 3.00 (-2.94,2.94) [-3.00,1.00]
Total: 24048 W: 3525 L: 3407 D: 17116

Bench changes at high depth.

Closes official-stockfish#1610
and    official-stockfish#1609

Bench: 4484288
CounterPly pushed a commit to CounterPly/Stockfish that referenced this pull request Jul 30, 2018
As discussed with @pb00068, the condition to prevent recursive verification
was not completely correct. This patch corrects that condition, and adds an
assert. In the current implementation, recursive verification needs to be
avoided in order not to break the verification of the move closer to the
root (i.e. to not set thisThread->nmp_min_ply to zero prematurely).

This patch is tested as a bug fix, based on and tested against PR official-stockfish#1609 .

passed STC:
http://tests.stockfishchess.org/tests/view/5b050f170ebc5914abc129c5
LLR: 2.95 (-2.94,2.94) [-3.00,1.00]
Total: 7875 W: 1669 L: 1523 D: 4683

passed LTC:
http://tests.stockfishchess.org/tests/view/5b0513970ebc5914abc129cd
LLR: 3.00 (-2.94,2.94) [-3.00,1.00]
Total: 24048 W: 3525 L: 3407 D: 17116

Bench changes at high depth.

Closes official-stockfish#1610
and    official-stockfish#1609

Bench: 4484288
CounterPly pushed a commit to CounterPly/Stockfish that referenced this pull request Sep 10, 2018
As discussed with @pb00068, the condition to prevent recursive verification
was not completely correct. This patch corrects that condition, and adds an
assert. In the current implementation, recursive verification needs to be
avoided in order not to break the verification of the move closer to the
root (i.e. to not set thisThread->nmp_min_ply to zero prematurely).

This patch is tested as a bug fix, based on and tested against PR official-stockfish#1609 .

passed STC:
http://tests.stockfishchess.org/tests/view/5b050f170ebc5914abc129c5
LLR: 2.95 (-2.94,2.94) [-3.00,1.00]
Total: 7875 W: 1669 L: 1523 D: 4683

passed LTC:
http://tests.stockfishchess.org/tests/view/5b0513970ebc5914abc129cd
LLR: 3.00 (-2.94,2.94) [-3.00,1.00]
Total: 24048 W: 3525 L: 3407 D: 17116

Bench changes at high depth.

Closes official-stockfish#1610
and    official-stockfish#1609

Bench: 4484288
CounterPly pushed a commit to CounterPly/Stockfish that referenced this pull request Oct 16, 2018
As discussed with @pb00068, the condition to prevent recursive verification
was not completely correct. This patch corrects that condition, and adds an
assert. In the current implementation, recursive verification needs to be
avoided in order not to break the verification of the move closer to the
root (i.e. to not set thisThread->nmp_min_ply to zero prematurely).

This patch is tested as a bug fix, based on and tested against PR official-stockfish#1609 .

passed STC:
http://tests.stockfishchess.org/tests/view/5b050f170ebc5914abc129c5
LLR: 2.95 (-2.94,2.94) [-3.00,1.00]
Total: 7875 W: 1669 L: 1523 D: 4683

passed LTC:
http://tests.stockfishchess.org/tests/view/5b0513970ebc5914abc129cd
LLR: 3.00 (-2.94,2.94) [-3.00,1.00]
Total: 24048 W: 3525 L: 3407 D: 17116

Bench changes at high depth.

Closes official-stockfish#1610
and    official-stockfish#1609

Bench: 4484288
CounterPly pushed a commit to CounterPly/Stockfish that referenced this pull request Oct 16, 2018
As discussed with @pb00068, the condition to prevent recursive verification
was not completely correct. This patch corrects that condition, and adds an
assert. In the current implementation, recursive verification needs to be
avoided in order not to break the verification of the move closer to the
root (i.e. to not set thisThread->nmp_min_ply to zero prematurely).

This patch is tested as a bug fix, based on and tested against PR official-stockfish#1609 .

passed STC:
http://tests.stockfishchess.org/tests/view/5b050f170ebc5914abc129c5
LLR: 2.95 (-2.94,2.94) [-3.00,1.00]
Total: 7875 W: 1669 L: 1523 D: 4683

passed LTC:
http://tests.stockfishchess.org/tests/view/5b0513970ebc5914abc129cd
LLR: 3.00 (-2.94,2.94) [-3.00,1.00]
Total: 24048 W: 3525 L: 3407 D: 17116

Bench changes at high depth.

Closes official-stockfish#1610
and    official-stockfish#1609

Bench: 4484288
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.

None yet

2 participants