Help with Debugging My Chess Engine
Moderators: hgm, Rebel, chrisw
-
- Posts: 53
- Joined: Mon Sep 19, 2016 6:51 am
-
- Posts: 793
- Joined: Sun Aug 03, 2014 4:48 am
- Location: London, UK
Re: Help with Debugging My Chess Engine
A set and an unordered set are very different.universecoder wrote:As you people suggest, I too don't think it's a problem with unordered_set, rather the problem might be that using it in a wrong way(the same way as a set?). For now, I'll use a set only.
A set requires objects it contains to be comparable. That's how it maintains ordering (usually using a tree).
An unordered_set requires objects to be hashable. It's usually implemented using a hash table.
If your type doesn't satisfy the strict weak ordering constraint (eg. you cannot have objects A, B, C, where A > B, B > C and C > A), a set won't work.
If your type doesn't satisfy the hash constraint (eg. you have the same object with different hashes), an unordered_set won't work.
Disclosure: I work for DeepMind on the AlphaZero project, but everything I say here is personal opinion and does not reflect the views of DeepMind / Alphabet.
-
- Posts: 12541
- Joined: Wed Mar 08, 2006 8:57 pm
- Location: Redmond, WA USA
Re: Help with Debugging My Chess Engine
Your solution is not good.
You found a way to reproduce a problem. This is a very good thing.
You should diagnose the exact problem and fix it.
It is not the hash table that was broken, but the code using it.
Switching techniques to a less efficient one, and then the problem vanishes is not a solution.
Until you actually understand the problem, and fix the real problem, all you are succeeding in is masking the trouble. It will raise its head again somewhere else.
You found a way to reproduce a problem. This is a very good thing.
You should diagnose the exact problem and fix it.
It is not the hash table that was broken, but the code using it.
Switching techniques to a less efficient one, and then the problem vanishes is not a solution.
Until you actually understand the problem, and fix the real problem, all you are succeeding in is masking the trouble. It will raise its head again somewhere else.
Taking ideas is not a vice, it is a virtue. We have another word for this. It is called learning.
But sharing ideas is an even greater virtue. We have another word for this. It is called teaching.
But sharing ideas is an even greater virtue. We have another word for this. It is called teaching.
-
- Posts: 4052
- Joined: Thu May 15, 2008 9:57 pm
- Location: Berlin, Germany
- Full name: Sven Schüle
Re: Help with Debugging My Chess Engine
As I wrote the OP simply uses an unordered set of integers representing square numbers in a piece list (one list per color and piece type). Integers are "hashable".matthewlai wrote:A set and an unordered set are very different.universecoder wrote:As you people suggest, I too don't think it's a problem with unordered_set, rather the problem might be that using it in a wrong way(the same way as a set?). For now, I'll use a set only.
A set requires objects it contains to be comparable. That's how it maintains ordering (usually using a tree).
An unordered_set requires objects to be hashable. It's usually implemented using a hash table.
If your type doesn't satisfy the strict weak ordering constraint (eg. you cannot have objects A, B, C, where A > B, B > C and C > A), a set won't work.
If your type doesn't satisfy the hash constraint (eg. you have the same object with different hashes), an unordered_set won't work.
The problem itself may arise simply by an out-of-bounds access to the pieceList[] array in a call like "pieceList[xy].erase(...)".
@Pranav:
I still believe that the undo code for the en passant square is wrong, it might even play a role in the crash scenario. What you do in undoMove() is (if the code from GitHub is still valid):
Code: Select all
enPassantSquare[!side] = EM;
int pawn = ( !side == white ) ? wp : bp;
int increment = ( !side == white ) ? ( 2*UP ) : ( 2*DOWN );
if ( move.currPiece == pawn && ( move.to == move.from + increment ) ) {
enPassantSquare[side] = move.from + increment/2;
//...
}
First thing is, you reset enPassantSquare[white] to EM (undefined), but how can you guess that? The previous black move might have been a pawn double step so prior to making this move enPassantSquare[white] may have been set, and this information gets lost that way.
Second, if you detect that the move to be taken back was a pawn double step then you set enPassantSquare[black] to the square that the pawn had crossed, which is correct *after* making a double step but not when taking it back. Here again, the en passant square that may have been set before making the move that led to the current position can't be derived from position+move only.
So it should be obvious that you need to store the (one!) ep square prior to making a move, same as castling rights.
-
- Posts: 53
- Joined: Mon Sep 19, 2016 6:51 am
Re: Help with Debugging My Chess Engine
I will look into the problem when I build the second version of this engine.
-
- Posts: 53
- Joined: Mon Sep 19, 2016 6:51 am
Re: Help with Debugging My Chess Engine
Ok, I will look into it while testing the engine.A little bit of testing right now supports my claim. Although I believe I that I've thought about this, you might be right. I am a little busy right now, so I'll properly think about what you said after some time. Thanks
-
- Posts: 53
- Joined: Mon Sep 19, 2016 6:51 am
Re: Help with Debugging My Chess Engine
Now that I read it, you are absolutely correct! I'll store the enPassant Square too.
@Pranav:
I still believe that the undo code for the en passant square is wrong, it might even play a role in the crash scenario. What you do in undoMove() is (if the code from GitHub is still valid):where at this point "side" is the opponent of the side for which you are undoing a move. Let's assume we are undoing a white move, so side == black.Code: Select all
enPassantSquare[!side] = EM; int pawn = ( !side == white ) ? wp : bp; int increment = ( !side == white ) ? ( 2*UP ) : ( 2*DOWN ); if ( move.currPiece == pawn && ( move.to == move.from + increment ) ) { enPassantSquare[side] = move.from + increment/2; //... }
First thing is, you reset enPassantSquare[white] to EM (undefined), but how can you guess that? The previous black move might have been a pawn double step so prior to making this move enPassantSquare[white] may have been set, and this information gets lost that way.
Second, if you detect that the move to be taken back was a pawn double step then you set enPassantSquare[black] to the square that the pawn had crossed, which is correct *after* making a double step but not when taking it back. Here again, the en passant square that may have been set before making the move that led to the current position can't be derived from position+move only.
So it should be obvious that you need to store the (one!) ep square prior to making a move, same as castling rights.