Small bug in Stockfish passed pawn evaluation?

Discussion of chess software programming and technical issues.

Moderators: bob, hgm, Harvey Williamson

Forum rules
This textbox is used to restore diagrams posted with the [d] tag before the upgrade.
Joost Buijs
Posts: 1035
Joined: Thu Jul 16, 2009 8:47 am
Location: Almere, The Netherlands

Small bug in Stockfish passed pawn evaluation?

Post by Joost Buijs » Thu Jul 09, 2015 6:50 am

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.

Joost Buijs
Posts: 1035
Joined: Thu Jul 16, 2009 8:47 am
Location: Almere, The Netherlands

Re: Small bug in Stockfish passed pawn evaluation?

Post by Joost Buijs » Thu Jul 09, 2015 7:42 am

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;


Joost Buijs
Posts: 1035
Joined: Thu Jul 16, 2009 8:47 am
Location: Almere, The Netherlands

Re: Small bug in Stockfish passed pawn evaluation?

Post by Joost Buijs » Thu Jul 09, 2015 8:04 am

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:

User avatar
JVMerlino
Posts: 1016
Joined: Wed Mar 08, 2006 9:15 pm
Location: San Francisco, California

Re: Small bug in Stockfish passed pawn evaluation?

Post by JVMerlino » Fri Jul 10, 2015 3:31 am

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

wgarvin
Posts: 838
Joined: Thu Jul 05, 2007 3:03 pm
Location: British Columbia, Canada

Re: Small bug in Stockfish passed pawn evaluation?

Post by wgarvin » Fri Jul 10, 2015 4:16 am

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. :)

Joerg Oster
Posts: 722
Joined: Fri Mar 10, 2006 3:29 pm
Location: Germany

Re: Small bug in Stockfish passed pawn evaluation?

Post by Joerg Oster » Fri Jul 10, 2015 9:25 am

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]" ...
Jörg Oster

Joost Buijs
Posts: 1035
Joined: Thu Jul 16, 2009 8:47 am
Location: Almere, The Netherlands

Re: Small bug in Stockfish passed pawn evaluation?

Post by Joost Buijs » Fri Jul 10, 2015 11:26 am

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.

User avatar
vittyvirus
Posts: 646
Joined: Wed Jun 18, 2014 12:30 pm
Full name: Fahad Syed

Re: Small bug in Stockfish passed pawn evaluation?

Post by vittyvirus » Fri Jul 10, 2015 1:54 pm

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.

kbhearn
Posts: 411
Joined: Thu Dec 30, 2010 3:48 am

Re: Small bug in Stockfish passed pawn evaluation?

Post by kbhearn » Fri Jul 10, 2015 3:06 pm

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.

Joost Buijs
Posts: 1035
Joined: Thu Jul 16, 2009 8:47 am
Location: Almere, The Netherlands

Re: Small bug in Stockfish passed pawn evaluation?

Post by Joost Buijs » Fri Jul 10, 2015 3:55 pm

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.

Post Reply