Page 1 of 1

Code review, anyone?

PostPosted: 27 Feb 2007, 17:02
by Tord Romstad
The following was also posted to the CCC.

Today I have released the source code of a new development version of Glaurung. This version isn't really interesting for end users - it is very much a work in progress. It is probably not quite as strong as Glaurung 1.2, and many features are missing (including parallel search, Windows support, and opening book). The reason I release this source code is two-fold: To obtain benchmarks for 64-bit CPUs (see another thread), and to try to get some knowledgeable opinions about the code. I have tried to rewrite everything since Glaurung 1.2, but I am not happy with the results. I'm beginning to realize that I couldn't write decent C/C++ code if my life depends on it.

I'm looking for some fellow chess programmer(s) who is willing to do a mutual code review. You study my code carefully, pointing out obvious bugs, possible improvements, and bad programming style. I do the same for your code. I hope that this will help both sides to improve their code. Does anyone want to try?

Needless to say, the strength of your program doesn't matter. I am equally interested in getting in touch with programmers at any levels. Your C/C++ skills are more important to me than the strength of your program. If your program is not open source, I will of course not share your code with anyone without your permission.

Tord

Re: Code review, anyone?

PostPosted: 27 Feb 2007, 18:06
by Fritz Reul
Hi Tord!

Please contact me via email to speak about your progress and some interesting smp stuff if you want!?

I have redesigned LOOP 100% with good results. But I am not satisfied with my recursive-alpha-beta-search. It should be possible to design a iterative search in order to

1. get a faster search
2. develop a faster and easier smp engine

Best to you
Fritz

Re: Code review, anyone?

PostPosted: 28 Feb 2007, 16:03
by Tord Romstad
Andrew Fan wrote:Just writing to let you know VS2003 doesn't like the new Glaurung source. Lots of compilation errors.

Hello Andrew,

That's no surprise. I even pointed out in the first point in this thread that Windows support is one of the many things I haven't implemented yet.
No time to fix them because I'm at work :P

No need to fix them - Windows support is not at all urgent. As long as the program is far from finished, there is no point in Windows binaries.

Tord

Re: Code review, anyone?

PostPosted: 28 Feb 2007, 22:53
by Zach Wegner
Fritz Reul wrote:Hi Tord!

Please contact me via email to speak about your progress and some interesting smp stuff if you want!?

I have redesigned LOOP 100% with good results. But I am not satisfied with my recursive-alpha-beta-search. It should be possible to design a iterative search in order to

1. get a faster search
2. develop a faster and easier smp engine

Best to you
Fritz


Don't expect an iterative search to be much faster at all. I doubt the speedup is measurable at all. The only reason to do it really is for SMP, and that is definitely not easy, trust me. But if you are interested I would be open to some DTS discussions.

Regards,
Zach

Re: Code review, anyone?

PostPosted: 01 Mar 2007, 00:34
by Gerd Isenberg
Hi Tord,

I had a short look at bitboard.h/cpp. Very nice, your style fits my taste. Ahh, you have your own magic in Matt Taylor's bitscan ;-)

I have that 32-K Between bitboard array as well in my 32-bit program and I fear it is likely the fastest solution.

My proposal based on absolute difference with only 1/2K lookup + shift left, might be a bit to expensive, computation- and register-wise, specially in 32-bit mode. It depends on how many L1 misses occur in total - whether the additional instructions eventually pay off (as always). Considering the boolean (0,1) sameRank expression, keeps the absolute rank deltas less 7, so that it doesn't collide with the lowest antidiagonal difference.

Code: Select all
extern u64 inbetweenByDiff[64];

// assumes sliding distance and sq1 != sq2

inline u64 inBetween(unsigned int sq1, unsigned int sq2) {
   int diff = sq2-sq1;
   int diffIfNegative = diff & (diff>>31);
   int sameRank = ((sq2^sq1)&~7) == 0;
   sq1 += diffIfNegative; // min
   sq2 -= diffIfNegative; // max
   return inbetweenByDiff[sq2-sq1-sameRank] << sq1;
}

Assume sq2 is 53 (f7) and sq1 is 26 (c4), the absolute difference is 3*9 = 27. inbetweenByDiff[27] = 0x40200 {9,18} or {b2,c3}, which becomes the desired 0x100800000000 {d5,e6} after shifting left 26.

Cheers,
Gerd

Re: Code review, anyone?

PostPosted: 01 Mar 2007, 02:16
by Dann Corbit
I made the changes needed for Win32.

Since my web site is dead as a doornail right now, I will have to send it by email.

As I recall, the changes were pretty trivial.

Re: Code review, anyone?

PostPosted: 01 Mar 2007, 09:13
by Daniel Shawul
Hi Tord,
I must say that is one deep c++ glaurung :)
I have reservations on implementing most of the c++features, but i think that there are some valuable advantages over c.
My first impression is that you used too many classes, even for things that you can do without. The smaller classes like COLOR and PIECE could be defined without classes IMO.
Wen you are done with this new glaurung, I am sure you will master c++.
Daniel

Re: Code review, anyone?

PostPosted: 01 Mar 2007, 09:52
by Tord Romstad
Hi Gerd!

Gerd Isenberg wrote:I had a short look at bitboard.h/cpp. Very nice, your style fits my taste.


Thanks!

Ahh, you have your own magic in Matt Taylor's bitscan ;-)


Of course - otherwise my program would have been a clone. :wink: All magic constants in my bitboard code are my own.

With regard to Matt Taylor's bitscan, I fear that it will be difficult for future programmers to avoid cloning. If I recall correctly, I proved that Matt's constant and mine were the two only possibilities.

I have that 32-K Between bitboard array as well in my 32-bit program and I fear it is likely the fastest solution.


I didn't like this solution very much, but it isn't very important: I currently don't use the between bitboards much. Thanks for your alternative solution, I'll give it a try (my computer is 32-bit, though).

Tord

Re: Code review, anyone?

PostPosted: 01 Mar 2007, 10:11
by Tord Romstad
Hello Daniel,

Thanks for your comments!
Daniel Shawul wrote:My first impression is that you used too many classes, even for things that you can do without. The smaller classes like COLOR and PIECE could be defined without classes IMO.

'Color' and 'Piece' are not classes, but enumerated types (in other words, nothing more than integers in disguise). My reason for using so many enums is to utilize the somewhat stronger typing of C++ to catch a bigger number of bugs at compile-time. The compiler won't allow me to pass a value of type Color to a function which expects a Piece as input, or to assign a value of type Square to a variable of type SquareDelta.

The disadvantage of this approach is that it forces me to write lots of ugly boilerplate code just to make it possible to use standard arithmetic operations on different types of enums. The advantage is that I will spend less time chasing bugs.

Wen you are done with this new glaurung, I am sure you will master c++.

That was never my goal - I still hope to avoid having to master C++. :)

Tord

Re: Code review, anyone?

PostPosted: 01 Mar 2007, 10:23
by Tord Romstad
Hi Dann,
Dann Corbit wrote:I made the changes needed for Win32.

Thanks, great!

Sending it to me is not at all urgent. The program is still months away from release in any case.

As I recall, the changes were pretty trivial.


That's good, but I'm afraid the hard part of the work still remains: I still haven't written the parallel search code, which will probably be the most difficult part of the program to port.

Tord

Re: Code review, anyone?

PostPosted: 01 Mar 2007, 11:49
by Pradu
Tord Romstad wrote:Hi Dann,
Dann Corbit wrote:I made the changes needed for Win32.

Thanks, great!

Sending it to me is not at all urgent. The program is still months away from release in any case.

As I recall, the changes were pretty trivial.


That's good, but I'm afraid the hard part of the work still remains: I still haven't written the parallel search code, which will probably be the most difficult part of the program to port.

Tord
It's easy if you create a cross-platform threading encapsulator using #ifdefs. This way #ifdefs won't actually appear in your code and only in the encapsulator. Here's the encapsulator I use for Buzz if it helps you get started. Also with this method you can get rid of the annoying windows.h include at the top of the source for Windows compiles.