Page 4 of 4

Re: Help with Debugging My Chess Engine

Posted: Thu Sep 29, 2016 8:08 pm
by universecoder
Ok.

Re: Help with Debugging My Chess Engine

Posted: Thu Sep 29, 2016 8:09 pm
by matthewlai
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 and an unordered set are very different.

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.

Re: Help with Debugging My Chess Engine

Posted: Thu Sep 29, 2016 8:34 pm
by Dann Corbit
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.

Re: Help with Debugging My Chess Engine

Posted: Thu Sep 29, 2016 9:04 pm
by Sven
matthewlai wrote:
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 and an unordered set are very different.

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.
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".

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;
                //...
        }
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.

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.

Re: Help with Debugging My Chess Engine

Posted: Fri Sep 30, 2016 2:30 pm
by universecoder
I will look into the problem when I build the second version of this engine.

Re: Help with Debugging My Chess Engine

Posted: Fri Sep 30, 2016 2:36 pm
by universecoder
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 :)

Re: Help with Debugging My Chess Engine

Posted: Fri Sep 30, 2016 8:23 pm
by universecoder

@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;
                //...
        }
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.

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.
Now that I read it, you are absolutely correct! I'll store the enPassant Square too.