I've gone through the RENAME/REFORMATTING thread and changed everything I could find, plus a few more. With this, let's close the previous issue and open another.
No functional change.
It does not appear to be not necessary or advantageous to
conditionally initialize kingRing[Us] or kingAttackersCount[Them],
so the 'else' can be removed.
STC
LLR: 2.96 (-2.94,2.94) [-3.00,1.00]
Total: 22873 W: 4923 L: 4804 D: 13146
http://tests.stockfishchess.org/tests/view/5be9a8270ebc595e0ae33c7e
No functional change
It looks like PawnsOnBothFlanks can be removed from initiative().
A barrage of tests seem to confirm that the adjustment to -110
does not gain elo to offset any potential loss by removing
PawnsOnBothFlanks.
STC
LLR: 2.96 (-2.94,2.94) [-3.00,1.00]
Total: 22014 W: 4760 L: 4639 D: 12615
http://tests.stockfishchess.org/tests/view/5b7f50cc0ebc5902bdbb3a3e
LTC
LLR: 2.96 (-2.94,2.94) [-3.00,1.00]
Total: 40561 W: 6667 L: 6577 D: 27317
http://tests.stockfishchess.org/tests/view/5b801f9f0ebc5902bdbb4467
The barrage of 0,4 tests on the -136 value are in my ps_tunetests branch.
http://tests.stockfishchess.org/tests/user/protonspring
Closes https://github.com/official-stockfish/Stockfish/pull/1751
Bench: 4413173
-------------
How to continue from there?
The fact that endgames with all the pawns on only one flank are
drawish is a well-known chess idea, so it seems quite strange that
this can be removed so easily without losing Elo.
In the past there had been attempts to improve on PawnsOnBothFlanks
with similar concepts (for instance using the pawn span value), but
the tests were at best neutral. Maybe Stockfish is now mature enough
that these refined ideas would work to replace PawnsOnBothFlanks?
This is a non-functional change. By pre-incrementing minKingPawnDistance
instead of post-incrementing, we can remove this -1.
This also makes DistanceRing more consistent with the rest of stockfish
since it now holds an actual "distance" instead of a less natural distance-1.
In current master, PseudoAttacks[KING][ksq] == DistanceRingBB[ksq][0]
With this patch, it will be PseudoAttacks[KING][ksq] == DistanceRingBB[ksq][1]
ie squares at distance 1 from the king. This is more natural use of distance.
The current array size DistanceRingBB[SQUARE_NB][8] is still OK with the new
definition, because maximum distance between two squares on a chess board is
seven (for example Kh1 and a8).
No functional change.
In current master, the function make_bitboard() does nothing apart from
helping initialize the SquareBB[] array. This seems like an unnecessary
abstraction layer.
The advantage of make_bitboard() is we can define a bitboard, in a simple
and general way, not only from a single square but also from a list of
squares. It is more elegant, faster and readable than combining multiple
SquareBB explicitly, but the last complex use case in evaluation was
simplified away a few months ago.
If make_bitboard() becomes useful again to define complicated bitboards,
it will be easy enough to reintroduce it using this pull request as
an implementation reference.
No functional change.
I believe using foward_file_bb() here is fewer instructions.
a) Fewer instructions and probably more clear (debatable).
b) Possible that a lookup is slower than a few local operations, but the
forward_file_bb table is probably used often enough that it is always
cached.
Passed
LLR: 2.96 (-2.94,2.94) [-3.00,1.00]
Total: 21004 W: 4263 L: 4141 D: 12600
http://tests.stockfishchess.org/tests/view/5b1cad830ebc5902ab9c6239
Closes https://github.com/official-stockfish/Stockfish/pull/1644
No functional change.
I was able to get this to pass which reduces BlockedByPawn to one dimension
with NO distance from edge offset.
GOOD) It's more simple and may provide additional clarity for further
simplifications. Facilitates migrating unblocked to one dimension as well.
BAD) If there is indeed a distance component to BlockedStorm (may or may
not be the case), this obfuscates this component into ShelterStrength and
UnblockedStorm. This may be more convoluted. Also, it may be more convenient
to have each of the three arrays (ShelterStrength, BlockedStorm, and UnBlocked)
be the same size.
STC:
LLR: 2.96 (-2.94,2.94) [-3.00,1.00]
Total: 96173 W: 19326 L: 19343 D: 57504
http://tests.stockfishchess.org/tests/view/5b04544d0ebc5914abc12965
LTC:
LLR: 2.95 (-2.94,2.94) [-3.00,1.00]
Total: 49818 W: 7441 L: 7363 D: 35014
http://tests.stockfishchess.org/tests/view/5b0487d50ebc5914abc12990
Closes https://github.com/official-stockfish/Stockfish/pull/1611
Bench: 5133208
The two lines of code in the patch seem to be just as good as master.
1. We now only look at the current square to see if it is currently backward,
whereas master looks there AND further ahead in the current file (master would
declare a pawn "backward" even though it could still safely advance a little).
This simplification allows us to avoid the use of the difficult logic with
`backmost_sq(Us, neighbours | stoppers)`.
2. The condition `relative_rank(Us,s) < RANK_5` is simplified away.
Passed STC:
LLR: 2.95 (-2.94,2.94) [-3.00,1.00]
Total: 68132 W: 14025 L: 13992 D: 40115
http://tests.stockfishchess.org/tests/view/5aedc97a0ebc5902a4099fd6
Passed LTC:
LLR: 2.95 (-2.94,2.94) [-3.00,1.00]
Total: 23789 W: 3643 L: 3527 D: 16619
http://tests.stockfishchess.org/tests/view/5aee4f970ebc5902a409a03a
Ideas for further work:
• The new code flags some pawns on the 5th rank as backward, which was not the
case in the old master. So maybe we should test a version with that included?
• Further tweaks of the backward condition with [0..5] bounds?
Closes https://github.com/official-stockfish/Stockfish/pull/1583
Bench: 5122789
Simplification: remove BlockedByKing from storm array and use a special rule.
The BlockedByKing section in the storm array is substantially similar to the
Unopposed section except for two extreme values V(-290), V(-274). Turns out
removing BlockedByKing and using a special rule for these two values shows
no Elo loss. All the other values in the BlockedByKing section are apparently
irrelevant. BlockedByKing now falls under unopposed which (to me) is a bit
more logical since there is no defending pawn on this file. Also, retuning
the Unopposed section may be another improvement.
GOOD) This is a simplification because the entire BlockedByKing section of
the storm array goes away reducing a few lines of code (and less values to
tune). This also brings clarity because the special rule is self documenting.
BAD) It takes execution time to apply the special rule. This should be negli-
gible because it is based on a template parameter and is boiled down to two
bitwise AND's.
STC:
LLR: 2.96 (-2.94,2.94) [-3.00,1.00]
Total: 33470 W: 6820 L: 6721 D: 19929
http://tests.stockfishchess.org/tests/view/5ae7b6e60ebc5926dba90e13
LTC:
LLR: 2.96 (-2.94,2.94) [-3.00,1.00]
Total: 47627 W: 7045 L: 6963 D: 33619
http://tests.stockfishchess.org/tests/view/5ae859ff0ebc5926dba90e85
Closes https://github.com/official-stockfish/Stockfish/pull/1574
Bench: 5512000
-----------
How to continue after this patch?
This patch may open the possibility to move the special rule to evaluate.cpp
in the evaluate::king() function, where we could refine the rule using king
danger information. For instance, with a king in H2 blocking an opponent pawn
in H3, it may be critical to know that the opponent has no safe check in G2
before giving the bonus :-)
This patch is non-functional. Current master does four operations to determine
whether an enemy pawn on this file is blocked by the king or not
```
f == file_of(ksq) && rkThem == relative_rank(Us, ksq) + 1 )
```
By adding a direction (based on the template color), this is reduced to two
operations. This works because b is limited to enemy pawns that are ahead of
the king and on the current file.
```
shift<Down>(b) & ksq
```
I've added a line of code, but the number of executing instructions is reduced
(I think). I'm not sure if this counts as a simplification, but it should
theoretically be a little faster (barely). The code line length is also reduced
making it a little easier to read.
Closes https://github.com/official-stockfish/Stockfish/pull/1552
No functional change.
"Loose pieces drop, in blitz keep everything protected"
Adding a small S(2,2) bonus for knights, bishops, rooks, and
queens that are "connected" to each other (in the sense that
they are under attack by our own pieces) apparently is a good
thing. It probably helps the pieces work together a bit better.
STC
LLR: 2.96 (-2.94,2.94) [0.00,5.00]
Total: 12317 W: 2655 L: 2467 D: 7195
http://tests.stockfishchess.org/tests/view/5aa2d86b0ebc590297cb6474
LTC
LLR: 2.96 (-2.94,2.94) [0.00,5.00]
Total: 35725 W: 5516 L: 5263 D: 24946
http://tests.stockfishchess.org/tests/view/5aa2fc6f0ebc590297cb64a8
How to continue from there (by Stefan Geschwentner)?
• First we should identify all other eval terms which have an overlap
with new connectivity bonus (like the outpost bonus). A simple way
would be subtract the connectivity bonus from them and look if this
better, or use a SPSA session for these terms.
• Tuning Connectivity himself with SPSA seems not so promising because
of the small range which is useful. Here manual testing changes of
Connectivity like +-1 seems better.
• The eg value is more important because in endgame the position gets
more open and so attacks on pieces are easier. Another important point
is that when defending/fortress-like positions each defending piece
needs a protection, otherwise attacks on them can break defense.
Closes https://github.com/official-stockfish/Stockfish/pull/1474
Bench: 5318575
Where variable names are explicitly incorrect, I feel morally obligated to at least
suggest an alternative. There are many, but these two are especially egregious.
No functional change.
As far as can tell, semiopenFiles are set if there is a pawn anywhere on
the file. The removed condition would be true even if the pawns were very
advanced, which doesn't make sense if we're looking for a trapped rook.
Seems the engine fairs better with this removed. My guess s that the
condition that mobility is 3 or less does this well enough.
Begs the question whether this is a mobility issue alone... not sure.
Should I do LTC test?
STC
LLR: 2.95 (-2.94,2.94) [-3.00,1.00]
Total: 13377 W: 3009 L: 2871 D: 7497
http://tests.stockfishchess.org/tests/view/5a855be40ebc590297cc8166
Passed LTC
LLR: 2.95 (-2.94,2.94) [-3.00,1.00]
Total: 16288 W: 2813 L: 2685 D: 10790
http://tests.stockfishchess.org/tests/view/5a8575a80ebc590297cc817e
Bench: 5006365
The current logic in master is to continue return quiet moves if their
history score is above 0. It appears as though this check can be
removed, which is also more logically consistent with the “skipQuiets”
semantics used in search.cpp.
This patch may open new opportunitiesto get Elo by changing or
tuning the definition of 'moveCountPruning' in line 830 of search.cpp,
because obeying skipQuiets without checking the history scores makes
the search more sensitive to 'moveCountPruning'.
STC
LLR: 2.96 (-2.94,2.94) [-3.00,1.00]
Total: 34780 W: 7680 L: 7584 D: 19516
http://tests.stockfishchess.org/tests/view/5a79f8d80ebc5902971a99db
LTC
LLR: 2.95 (-2.94,2.94) [-3.00,1.00]
Total: 38757 W: 6732 L: 6641 D: 25384
http://tests.stockfishchess.org/tests/view/5a7afebe0ebc5902971a9a46
Bench 4954595
For these recaptures, we’re are only considering those captures
that recapture the recapture square (small portion of all the
captures). Therefore, scoring all of the captures and pick_besting
out of the whole group is not necessary.
STC
LLR: 2.96 (-2.94,2.94) [-3.00,1.00]
Total: 85583 W: 18978 L: 18983 D: 47622
http://tests.stockfishchess.org/tests/view/5a717faa0ebc590f2c86e9a7
LTC
LLR: 2.96 (-2.94,2.94) [-3.00,1.00]
Total: 20231 W: 3533 L: 3411 D: 13287
http://tests.stockfishchess.org/tests/view/5a73ad330ebc5902971a96ba
Bench: 5023593
The difference between QCAPTURES_1 and QCAPTURES_2 quiescence search stages
boils down to a simple check of depth. The way it's being done now is
unnecessarily complex.
This patch is simpler, clearer, and easier to understand.
Passed SPRT[-3..1] test at STC:
LLR: 2.95 (-2.94,2.94) [-3.00,1.00]
Total: 99755 W: 22158 L: 22192 D: 55405
http://tests.stockfishchess.org/tests/view/5a71f41c0ebc590f2c86e9cb
No functional change.
