Code review, anyone?

Programming Topics (Computer Chess) and technical aspects as test techniques, book building, program tuning etc

Moderator: Andres Valverde

Code review, anyone?

Postby Tord Romstad » 27 Feb 2007, 17:02

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
User avatar
Tord Romstad
 
Posts: 639
Joined: 09 Oct 2004, 12:49
Location: Oslo, Norway

Re: Code review, anyone?

Postby Fritz Reul » 27 Feb 2007, 18:06

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
Fritz Reul
 
Posts: 7
Joined: 15 Sep 2006, 18:44

Re: Code review, anyone?

Postby Tord Romstad » 28 Feb 2007, 16:03

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
User avatar
Tord Romstad
 
Posts: 639
Joined: 09 Oct 2004, 12:49
Location: Oslo, Norway

Re: Code review, anyone?

Postby Zach Wegner » 28 Feb 2007, 22:53

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
User avatar
Zach Wegner
 
Posts: 182
Joined: 26 Sep 2004, 22:02
Location: Austin, Texas, USA

Re: Code review, anyone?

Postby Gerd Isenberg » 01 Mar 2007, 00:34

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
Gerd Isenberg
 
Posts: 285
Joined: 31 Jan 2005, 20:31
Location: Hattingen, Germany

Re: Code review, anyone?

Postby Dann Corbit » 01 Mar 2007, 02:16

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.
Dann Corbit
 

Re: Code review, anyone?

Postby Daniel Shawul » 01 Mar 2007, 09:13

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
User avatar
Daniel Shawul
 
Posts: 366
Joined: 28 Sep 2004, 09:33
Location: Ethiopia

Re: Code review, anyone?

Postby Tord Romstad » 01 Mar 2007, 09:52

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
User avatar
Tord Romstad
 
Posts: 639
Joined: 09 Oct 2004, 12:49
Location: Oslo, Norway

Re: Code review, anyone?

Postby Tord Romstad » 01 Mar 2007, 10:11

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
User avatar
Tord Romstad
 
Posts: 639
Joined: 09 Oct 2004, 12:49
Location: Oslo, Norway

Re: Code review, anyone?

Postby Tord Romstad » 01 Mar 2007, 10:23

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
User avatar
Tord Romstad
 
Posts: 639
Joined: 09 Oct 2004, 12:49
Location: Oslo, Norway

Re: Code review, anyone?

Postby Pradu » 01 Mar 2007, 11:49

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.
User avatar
Pradu
 
Posts: 343
Joined: 12 Jan 2005, 19:17
Location: Chandler, Arizona, USA


Return to Programming and Technical Discussions

Who is online

Users browsing this forum: No registered users and 9 guests