I want experts to look at my c-source for hashtables

[Dann Corbit]

Interesting -- In my engine, I just store the hash key at each position and when I unmake a move, I just grab the old hash key, so I never have errors in unmake. Is there some reason why this is a bad idea?

It's the right solution in the long run, but I think it is a mistake when you are first writing the engine.

How will you be sure that you did not forget to undo something (unless you store the whole board before make).

[MattieShoes]

Interesting -- In my engine, I just store the hash key at each position and when I unmake a move, I just grab the old hash key, so I never have errors in unmake. Is there some reason why this is a bad idea?

It's the right solution in the long run, but I think it is a mistake when you are first writing the engine.

How will you be sure that you did not forget to undo something (unless you store the whole board before make).

Maybe I'm missing something... How does incrementally un-updating the hash key in unmake help you find bugs? Since my code never undoes the hash key at all, just restoring it from a stack, there's zero bugs in the hash code in unmake. It consists of something like
hashKey = hist->hashKey;
Any errors HAVE to have been introduced in the makemove function since that's the only place where I actually incrementally update the hash key.

Regarding the actual board representation, I worked the bugs out on that with perft numbers and a simple sanity check function for piece lists and whatnot before ever implementing hashing.

I apologize if I'm missing something obvious, but I honestly don't see the benefit even for testing.

[ffao]

Dann might be referring to something like this:

Code: Select all

/* rollback en passant move */
if (move_type == en_passant) {
    add_piece(opponent_pawn, enpassant_square);
    zobrist ^= PIECE_HASH[opponent_pawn][enpassant_square];
}

If you had forgotten to undo the enpassant capture, the resulting hash key would be off (although I agree that perft is a better place to catch this).

Also, perhaps not as obviously, if you had forgotten to update the hash in makemove for an en passant capture, the resulting hash key would also be off.

So, basically, you're doing it twice and checking that results match (if they don't, there is an error either in make or in takeback), so that an error has to slip by in both functions. It's much more likely you'll make a mistake, but it's less likely it's going to stay there unnoticed.

If you just grab the past hash in unmake, the hashes will never be off, which might hide bugs in the hashing function. And, of course, bugs that don't show themselves cause much more damage.

If you don't do this, the only way to test hashing is to try transpositions. With such an incremental scheme you also have the option of making/taking back lots of moves and checking that the keys match.

Of course, that's a debugging strategy. Doesn't mean that the final version can't simply track past keys (which is also what I do).

[mcostalba]

Interesting -- In my engine, I just store the hash key at each position and when I unmake a move, I just grab the old hash key, so I never have errors in unmake. Is there some reason why this is a bad idea?

Because unmake by copying back it is a waste of time.


Glaurung also uses this way, but in Stockfish 1.3 I have greatly enanched undo_move(), there is no copy back of values, just a pointer switch....undo_move() times are cutted in more then half.

Interested people can look at the sources

[Sven]

I downloaded your source from JA's site and compiled it with VC++ Express 2005. The source tells it is version "1.80d JA" and the build date is "08-06-09".

Apart from two compile errors which were trivial to fix (lines 4033, 4181: add an empty statement ";" since a closing bracket "}" may not follow a label), MSVC++ emits many warnings. I strongly recommend to fix these!

The most serious one is related to the last line of this section:

Code: Select all

void opdat_hash(signed char fr, signed char to, signed int score, signed char ifinal)
{  signed char final, surprise, zz;
   if (/* 22/12ply_1<=plydyb  and */ swhash>0      and p&#91;ply&#93;.swnullhash==0
		            and score<=&#40;int_max&#41; and score>>&#40;int_min&#41;     // to avoid uninitialised mms-values......
&#91;...&#93;

This is code beginning in line 6583 (originally 6581) of dabbaba.c. MSVC++ 2005 correctly prints this warning:

Code: Select all

warning C4293: '>>' : shift count negative or too big, undefined behavior

Probably the ">>" should be ">="? That would make more sense since "score>>(-30000)" is not what you want to have (I guess it's always zero which might lead to never entering the code after the "if" )

Hope that helps a little bit, although the "hash error" messages at startup time do still appear after fixing this.

Sven

[ffao]

for (hz=0; hz<=7; ++hz) *(hashint+hz)=*(farp+key*8+hz);

do you really have to do a complex loop to copy a piece of memory?

If I understand the code correctly, then this should do the same as the loop...

Code: Select all

memcpy(hashint, farp+key*8, sizeof(int)*8);

Also, do you keep track of castling status in your hash keys?

[Sven]

Hope that helps a little bit, although the "hash error" messages at startup time do still appear after fixing this.

The messages go away if I set "*use_hashtables 1" in "dabbaba.ini".

I tried to provide an ini file containing only that single line, which crashed in ReadINIFile(). Changing NULL into "" (empty string) in line 888 helped, so I recommend that you fix this.

The code is very difficult to read for me for at least these reasons:
- chaotic indentation policy (indeed, no policy)
- too much unused code left within comments (I'd better remove it, while archiving old versions)
- sometimes too many empty lines, sometimes not enough empty lines
- too often many statements on the same line (o.k., others do this too sometimes but it should be kept on an acceptable level)
- frequent use of goto, often instead of simple for loop
- use of macros like "#define equal ==" makes life hard for C programmers ...

So if your intention is to improve your program then you most probably want to reduce the number of bugs in your code. One requirement to achieve this is that you can read and understand your code. Of course the fact that I have difficulties reading it does not mean that you have, too, but from what you wrote I guess there are at least some parts that you don't fully understand.

Therefore I want to encourage you to improve readability. The code is not bad at all but all I want to say is that there is much potential for improvement.

Sven

[Sven]

In the first line of function b_check_directions(), variable met_big_piece1 is not initialized (others aren't, too, but this one implies a problem). This leads to a compiler warning at the place where the variable is used for the first time. The same does not appear in w_check_directions().

Sven

[JensBNielsen]

Hi Sven

Thanks for your many comments, your proposals and the time you have taken to look at Dabbaba.

I actually discovered the >> error by reading my own post here. It should be >= instead.

Thanks for discovering the difference in w_check_direction and b_check_direction. How did you discover that?!

The published version has a little error in the dabbaba.ini file:
Use of hashtables (swhash) should be 1:
0=don't use hashtables (gives errors when checking the hashtables - I should remove that)
1=use hashtables in prevalue of moves (the moves found in the hashtables should be tried first). But Dabbaba fails to get many succesful reads in the hashtables, that could be used for a better prevalue of moves...
7=full use og hashtables (also use the retrieved scores for cut-off etc - but my tests indicate some bad moves are played, so I don't think this works properly)

First I would just like the hashtables to work properly.
Then I would like my code to be clean and straight, but I would also like to add new knowledge and ideas - and I have a lot!

Best, Jens

[JensBNielsen]

Thanks; I will try that memcpy...

The hashkey might have a little problem concerning castling, but not enough to explain the error I generally have with the hashtables!
But I better make it 100% concerning castling!

Best, Jens