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

Minor threats b #13

Closed
wants to merge 2 commits into from
Closed

Minor threats b #13

wants to merge 2 commits into from

Conversation

ajithcj
Copy link

@ajithcj ajithcj commented Jun 29, 2014

Add bonuses for Minors attacking enemy pieces(except pawns) even when they are protected by enemy pawns.

Patch has passed both STC
LLR: 2.95 (-2.94,2.94) [-1.50,4.50]
Total: 8206 W: 1426 L: 1304 D: 5476

and LTC
LLR: 2.97 (-2.94,2.94) [0.00,6.00]
Total: 19534 W: 2821 L: 2640 D: 14073

Note: LTC was run at SPRT [0,6] as it is not a complicated change.

@ajithcj
Copy link
Author

ajithcj commented Jun 29, 2014

There were several previous tests that tried to pick the highest valued piece(for WeakEnemies). But all those patches failed.
I picked lsb as weakEnemies was also computed in the same way and somehow using lsb was the most successful approach so far.

@ajithcj
Copy link
Author

ajithcj commented Jun 29, 2014

Morover the value of two threats is not always the sum of two threats. For example I would think that when there are two threats the resultant bonus should never exceed ThreatenedByPawn[] bonus.
That way a piece doesn't get threatened by a pawn to create threats on two minor pieces.

@zamar
Copy link

zamar commented Jun 29, 2014

I need to think about this patch. It adds some complexity, so it might be a good idea to measure its value in ELO before committing.

@glinscott
Copy link
Contributor

Interesting idea! I think this would be clearer without adding ei.attackedByMinors. Most of the code paths where it's used are behind if conditions, so it might not be a speed up as well. Can you push an update without that? Then it should be quite a small diff. Thanks!

@ajithcj
Copy link
Author

ajithcj commented Jun 30, 2014

Gary, I had tested it without adding ei.attackedByMinors
The test was minor_threats and it failed yellow at STC:
http://tests.stockfishchess.org/tests/view/53ae767b0ebc5926dec5ce10

ei.attackedByMinors is an important part of this patch as it avoids replicating Minor attacks by Bishop and Knight at 5 different places in the source code.

@glinscott
Copy link
Contributor

That could have just been bad luck though. Have you measured ei.attackedByMinors to be a speed-up?

@ajithcj
Copy link
Author

ajithcj commented Jun 30, 2014

No I haven't. I have to read Marco's instructions on testing speedup. Will test it later when I have time. Have to leave for work.

@lucasart
Copy link

@ajithcj: ok. I will try to remove these lsb once this patch is commited. Problem might be that it requires retuning...

@lucasart
Copy link

@zamar: the added complexity is small. once you remove the attackedByMinors stuff, which is non functional code refactoring, the diff is clean and there is really one idea in this patch (not a mix). it already passed sprt(0,6) at ltc after an sprt(-1.5,4.5) preselection at stc. i don't think it's a good idea to add some ad hoc third testing step after that.

@ajithcj
Copy link
Author

ajithcj commented Jun 30, 2014

Joona/Gary, feel free to launch an elo gain test if you feel the need.

@zamar
Copy link

zamar commented Jun 30, 2014

OK, attackedByMinors stuff lead me astray. If the added complexity is small, it's fine for me to merge this. But whoever merges this, please remove the attackedByMinors stuff while merging, I don't think that it provides any real value... If it is a speed up it can be tested separately. Thanks.

@ajithcj
Copy link
Author

ajithcj commented Jun 30, 2014

Joona, as I said in my comment to Gary above, attackedByMinors is a simplification which works very well with this patch. Without it You are effectively committing an inefficient patch as shown by below test(without attackedByMinors);
http://tests.stockfishchess.org/tests/view/53ae767b0ebc5926dec5ce10
I understand the need to separate performance gains from functional changes. But these are not two separate ideas and making a patch inefficient to separate out the performance changes doesn't seem to make sense.
I'm not sure the minor_threats_B change or attackedByMinors changes will separately pass SPRT(0,6) simply because they complement each other well.

glinscott referenced this pull request Jun 30, 2014
… they are protected by enemy pawns.

Patch passed STC
LLR: 2.95 (-2.94,2.94) [-1.50,4.50]
Total: 8206 W: 1426 L: 1304 D: 5476

and LTC
LLR: 2.97 (-2.94,2.94) [0.00,6.00]
Total: 19534 W: 2821 L: 2640 D: 14073

Bench: 9942172
@glinscott
Copy link
Contributor

Merged in 6b35430.

@ajithcj, I tested with and without the attackedByMinors change. It was slightly faster without the array, so I haven't committed it.

With attackedByMinors
Total time (ms) : 4585
Nodes searched : 9942172
Nodes/second : 2168412

Without attackedByMinors
Total time (ms) : 4561
Nodes searched : 9942172
Nodes/second : 2179822

@glinscott glinscott closed this Jun 30, 2014
@ajithcj
Copy link
Author

ajithcj commented Jun 30, 2014

OK. Thanks.

niklasf pushed a commit to niklasf/Stockfish that referenced this pull request Jun 29, 2015
Exclude queen from Rook Contact Check computation
Alayan-stk-2 pushed a commit to Alayan-stk-2/Stockfish that referenced this pull request Feb 3, 2020
theo77186 pushed a commit to theo77186/Stockfish that referenced this pull request Jun 24, 2020
@vondele vondele mentioned this pull request May 4, 2022
Viren6 added a commit to Viren6/Stockfish that referenced this pull request Feb 7, 2024
bench 1024 1 26 default depth nnue

Hit #5: Total 670979 Hits 290935 Hit Rate (%) 43.3598
Hit #6: Total 526362 Hits 200666 Hit Rate (%) 38.1232
Hit #7: Total 409913 Hits 155327 Hit Rate (%) 37.8927
Hit #8: Total 307992 Hits 107444 Hit Rate (%) 34.8853
Hit official-stockfish#9: Total 238620 Hits 82639 Hit Rate (%) 34.6321
Hit official-stockfish#10: Total 159371 Hits 52820 Hit Rate (%) 33.1428
Hit official-stockfish#11: Total 109867 Hits 35774 Hit Rate (%) 32.5612
Hit official-stockfish#12: Total 79281 Hits 24117 Hit Rate (%) 30.4196
Hit official-stockfish#13: Total 60737 Hits 18658 Hit Rate (%) 30.7193
Hit official-stockfish#14: Total 44693 Hits 13464 Hit Rate (%) 30.1255
Hit official-stockfish#15: Total 35225 Hits 11088 Hit Rate (%) 31.4776
Hit official-stockfish#16: Total 50823 Hits 16387 Hit Rate (%) 32.2433
Hit official-stockfish#17: Total 19747 Hits 6494 Hit Rate (%) 32.886
Hit official-stockfish#18: Total 15972 Hits 5266 Hit Rate (%) 32.9702
Hit official-stockfish#19: Total 12658 Hits 4173 Hit Rate (%) 32.9673
Hit official-stockfish#20: Total 8633 Hits 2871 Hit Rate (%) 33.2561
Hit official-stockfish#21: Total 6368 Hits 2236 Hit Rate (%) 35.1131
Hit official-stockfish#22: Total 4569 Hits 1670 Hit Rate (%) 36.5507
Hit official-stockfish#23: Total 3136 Hits 1302 Hit Rate (%) 41.5179
Hit official-stockfish#24: Total 1992 Hits 910 Hit Rate (%) 45.6827
Hit official-stockfish#25: Total 1398 Hits 680 Hit Rate (%) 48.6409
Hit official-stockfish#26: Total 883 Hits 514 Hit Rate (%) 58.2106
Hit official-stockfish#27: Total 569 Hits 348 Hit Rate (%) 61.1599
Hit official-stockfish#28: Total 331 Hits 223 Hit Rate (%) 67.3716
Hit official-stockfish#29: Total 205 Hits 141 Hit Rate (%) 68.7805
Hit official-stockfish#30: Total 103 Hits 71 Hit Rate (%) 68.932
Hit official-stockfish#31: Total 57 Hits 34 Hit Rate (%) 59.6491
Viren6 added a commit to Viren6/Stockfish that referenced this pull request Feb 7, 2024
bench 1024 1 26 default depth nnue

Hit #5: Total 673841 Hits 286569 Hit Rate (%) 42.5277
Hit #6: Total 563135 Hits 213101 Hit Rate (%) 37.8419
Hit #7: Total 454918 Hits 175263 Hit Rate (%) 38.5263
Hit #8: Total 349255 Hits 123193 Hit Rate (%) 35.2731
Hit official-stockfish#9: Total 271301 Hits 96581 Hit Rate (%) 35.5992
Hit official-stockfish#10: Total 184972 Hits 64415 Hit Rate (%) 34.8242
Hit official-stockfish#11: Total 127732 Hits 43448 Hit Rate (%) 34.015
Hit official-stockfish#12: Total 93881 Hits 30636 Hit Rate (%) 32.6328
Hit official-stockfish#13: Total 73504 Hits 24436 Hit Rate (%) 33.2444
Hit official-stockfish#14: Total 54409 Hits 17552 Hit Rate (%) 32.2594
Hit official-stockfish#15: Total 43262 Hits 13924 Hit Rate (%) 32.1853
Hit official-stockfish#16: Total 63010 Hits 20206 Hit Rate (%) 32.0679
Hit official-stockfish#17: Total 23513 Hits 7507 Hit Rate (%) 31.927
Hit official-stockfish#18: Total 19250 Hits 6361 Hit Rate (%) 33.0442
Hit official-stockfish#19: Total 15187 Hits 4775 Hit Rate (%) 31.4414
Hit official-stockfish#20: Total 10790 Hits 3435 Hit Rate (%) 31.835
Hit official-stockfish#21: Total 7924 Hits 2486 Hit Rate (%) 31.373
Hit official-stockfish#22: Total 5587 Hits 1941 Hit Rate (%) 34.7414
Hit official-stockfish#23: Total 3822 Hits 1314 Hit Rate (%) 34.3799
Hit official-stockfish#24: Total 2559 Hits 987 Hit Rate (%) 38.5698
Hit official-stockfish#25: Total 1568 Hits 623 Hit Rate (%) 39.7321
Hit official-stockfish#26: Total 1042 Hits 491 Hit Rate (%) 47.1209
Hit official-stockfish#27: Total 651 Hits 370 Hit Rate (%) 56.8356
Hit official-stockfish#28: Total 455 Hits 257 Hit Rate (%) 56.4835
Hit official-stockfish#29: Total 235 Hits 135 Hit Rate (%) 57.4468
Hit official-stockfish#30: Total 131 Hits 86 Hit Rate (%) 65.6489
Hit official-stockfish#31: Total 41 Hits 35 Hit Rate (%) 85.3659
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.

4 participants