Page 1 of 2

Small bug in Stockfish passed pawn evaluation?

Posted: Thu Jul 09, 2015 6:50 am
by Joost Buijs
This morning I was looking at the Stockfish passed pawn evaluation and I saw the following:

Code: Select all


else if (pos.pieces(Us) & blockSq)
mbonus += rr * 3 + r * 2 + 3, ebonus += rr + r * 2;

A bitboard containing our pieces is bitwise anded with the block square which is an enum c.q. integer, should this not be anded with the bitmask of the blocksquare?
Maybe I'm mistaken, it is possible that I don't understand the code.

Re: Small bug in Stockfish passed pawn evaluation?

Posted: Thu Jul 09, 2015 7:42 am
by Joost Buijs
I see the same thing happening somewhat earlier in the same piece of code.

Code: Select all


// If there aren't any enemy attacks, assign a big bonus. Otherwise
// assign a smaller bonus if the block square isn't attacked.
int k = !unsafeSquares ? 15 : !(unsafeSquares & blockSq) ? 9 : 0;


else if (defendedSquares & blockSq)
k += 4;


Re: Small bug in Stockfish passed pawn evaluation?

Posted: Thu Jul 09, 2015 8:04 am
by Joost Buijs
Ah, I now see that there is an & operator defined in the bitboard class which take a square number as one of the arguments.

This is a nice way to create unreadable code, one of the advantages of C++! :shock:

Re: Small bug in Stockfish passed pawn evaluation?

Posted: Fri Jul 10, 2015 3:31 am
by JVMerlino
Joost Buijs wrote:Ah, I now see that there is an & operator defined in the bitboard class which take a square number as one of the arguments.

This is a nice way to create unreadable code, one of the advantages of C++! :shock:
I can't put into words how much I hate that sort of thing. I defy the Stockfish team to show that the redefinition of that operator adds any ELO.

:P

jm

Re: Small bug in Stockfish passed pawn evaluation?

Posted: Fri Jul 10, 2015 4:16 am
by wgarvin
I dislike this trick too. It violates the "Principle of Least Surprise". Programmers who haven't read this bitboard class should expect & to be a bitwise-and of the two values. They should always be able to safely assume that--this trick would surprise them, and is therefore bad. :)

Re: Small bug in Stockfish passed pawn evaluation?

Posted: Fri Jul 10, 2015 9:25 am
by Joerg Oster
wgarvin wrote:I dislike this trick too. It violates the "Principle of Least Surprise". Programmers who haven't read this bitboard class should expect & to be a bitwise-and of the two values. They should always be able to safely assume that--this trick would surprise them, and is therefore bad. :)
That's quite funny, because for me as a non-programmer this was rather easy to understand.
And I find it far better readable code than to always write "SquareBB[square]" ...

Re: Small bug in Stockfish passed pawn evaluation?

Posted: Fri Jul 10, 2015 11:26 am
by Joost Buijs
Joerg Oster wrote: That's quite funny, because for me as a non-programmer this was rather easy to understand.
And I find it far better readable code than to always write "SquareBB[square]" ...
For a programmer it is just counterintuitive, and it doesn't serve any purpose.
The & operator is defined to do a bitwise and, when you overload it and give it a different meaning this is very confusing.

Re: Small bug in Stockfish passed pawn evaluation?

Posted: Fri Jul 10, 2015 1:54 pm
by vittyvirus
Joost Buijs wrote:
Joerg Oster wrote: That's quite funny, because for me as a non-programmer this was rather easy to understand.
And I find it far better readable code than to always write "SquareBB[square]" ...
For a programmer it is just counterintuitive, and it doesn't serve any purpose.
The & operator is defined to do a bitwise and, when you overload it and give it a different meaning this is very confusing.
Infact it's very useful.
1. It helps in writing short, concise code.
2. There can be bugs (and I've seen them) like doing x & sq instead of x & SqMask[sq]
3. It's extensible. For example, 1ULL << sq might be faster than SqMask[sq]. You just need to change few lines of code.
4. There are intrinsics for checking whether a given bit is set or not. You only need to change couple of lines of code to use those intrinsics.

Re: Small bug in Stockfish passed pawn evaluation?

Posted: Fri Jul 10, 2015 3:06 pm
by kbhearn
vittyvirus wrote:
Joost Buijs wrote:
Joerg Oster wrote: That's quite funny, because for me as a non-programmer this was rather easy to understand.
And I find it far better readable code than to always write "SquareBB[square]" ...
For a programmer it is just counterintuitive, and it doesn't serve any purpose.
The & operator is defined to do a bitwise and, when you overload it and give it a different meaning this is very confusing.
Infact it's very useful.
1. It helps in writing short, concise code.
2. There can be bugs (and I've seen them) like doing x & sq instead of x & SqMask[sq]
3. It's extensible. For example, 1ULL << sq might be faster than SqMask[sq]. You just need to change few lines of code.
4. There are intrinsics for checking whether a given bit is set or not. You only need to change couple of lines of code to use those intrinsics.
all of those advantages can be gained with a named member function instead of an operator, with none of the downside of breaking expected functionality of an operator.

Re: Small bug in Stockfish passed pawn evaluation?

Posted: Fri Jul 10, 2015 3:55 pm
by Joost Buijs
kbhearn wrote:
vittyvirus wrote:
Joost Buijs wrote:
Joerg Oster wrote: That's quite funny, because for me as a non-programmer this was rather easy to understand.
And I find it far better readable code than to always write "SquareBB[square]" ...
For a programmer it is just counterintuitive, and it doesn't serve any purpose.
The & operator is defined to do a bitwise and, when you overload it and give it a different meaning this is very confusing.
Infact it's very useful.
1. It helps in writing short, concise code.
2. There can be bugs (and I've seen them) like doing x & sq instead of x & SqMask[sq]
3. It's extensible. For example, 1ULL << sq might be faster than SqMask[sq]. You just need to change few lines of code.
4. There are intrinsics for checking whether a given bit is set or not. You only need to change couple of lines of code to use those intrinsics.
all of those advantages can be gained with a named member function instead of an operator, with none of the downside of breaking expected functionality of an operator.
Indeed, a member function or even a simple macro can do the same without the need to redefine an operator.