RuyDos publicly available

[Guenther]

Hi,

I am making RuyDos publicly available: https://bitbucket.org/alonamaloh/ruydos

There are two things I would like help with:
1) Does it work for you? In particular I am curious if the Windows version works, because I don't have Windows anywhere and I compiled it from Linux.

Thanks!

Thanks for releasing it. The plain 64bit compile runs on my no popcount Win7 quadcore w/o problems so far.

I guess you'll add some uci options in the future?
Will you also add an internal version numbering scheme?
I have currently added it to my chronology with the generic hex number of the master release.

[AlvaroBegue]

Hi,

I am making RuyDos publicly available: https://bitbucket.org/alonamaloh/ruydos

There are two things I would like help with:
1) Does it work for you? In particular I am curious if the Windows version works, because I don't have Windows anywhere and I compiled it from Linux.

Thanks!

Thanks for releasing it. The plain 64bit compile runs on my no popcount Win7 quadcore w/o problems so far.

I guess you'll add some uci options in the future?
Will you also add an internal version numbering scheme?
I have currently added it to my chronology with the generic hex number of the master release.

Those are both very reasonable steps. I'll get to them. I've been lazy about figuring out how UCI options work, but it can't be that hard. At the very least I need to be able to adjust the hash size, I guess.

Thanks!

[Dann Corbit]

Hi,

I am making RuyDos publicly available: https://bitbucket.org/alonamaloh/ruydos

There are two things I would like help with:
1) Does it work for you? In particular I am curious if the Windows version works, because I don't have Windows anywhere and I compiled it from Linux.

Thanks!

Thanks for releasing it. The plain 64bit compile runs on my no popcount Win7 quadcore w/o problems so far.

I guess you'll add some uci options in the future?
Will you also add an internal version numbering scheme?
I have currently added it to my chronology with the generic hex number of the master release.

Those are both very reasonable steps. I'll get to them. I've been lazy about figuring out how UCI options work, but it can't be that hard. At the very least I need to be able to adjust the hash size, I guess.

Thanks!

Look at how Stockfish does UCI.
It's beautiful and brilliant.
Even a peanut-head like me can add new options at a whim.

[flok]

Hi,

I suggest compiling it with the -ggdb3 flag and running it from valgrind.
This shows up quite a few uninitialized-but-used variables:

Code: Select all

==2128== Conditional jump or move depends on uninitialised value(s)
==2128==    at 0x11532C: Engine::print_best_line(int) [clone .constprop.9] (hash.cpp:290)
==2128==    by 0x10F324: Engine::search_root() (engine.cpp:1132)
==2128==    by 0x11A312&#58; UCI&#58;&#58;go&#40;std&#58;&#58;__cxx11&#58;&#58;basic_istringstream<char, std&#58;&#58;char_traits<char>, std&#58;&#58;allocator<char> >&) &#40;engine.cpp&#58;965&#41;
==2128==    by 0x11267E&#58; UCI&#58;&#58;process&#40;std&#58;&#58;__cxx11&#58;&#58;basic_string<char, std&#58;&#58;char_traits<char>, std&#58;&#58;allocator<char> >) &#40;uci.cpp&#58;217&#41;
==2128==    by 0x10C059&#58; main &#40;main.cpp&#58;23&#41;

==2128== Conditional jump or move depends on uninitialised value&#40;s&#41;
==2128==    at 0x114304&#58; Engine&#58;&#58;negamax&#40;int, int, int, int&#41; &#91;clone .part.121&#93; &#40;hash.cpp&#58;349&#41;
==2128==    by 0x10F4F9&#58; Engine&#58;&#58;search_root&#40;) &#40;engine.cpp&#58;805&#41;
==2128==    by 0x11A312&#58; UCI&#58;&#58;go&#40;std&#58;&#58;__cxx11&#58;&#58;basic_istringstream<char, std&#58;&#58;char_traits<char>, std&#58;&#58;allocator<char> >&) &#40;engine.cpp&#58;965&#41;
==2128==    by 0x11267E&#58; UCI&#58;&#58;process&#40;std&#58;&#58;__cxx11&#58;&#58;basic_string<char, std&#58;&#58;char_traits<char>, std&#58;&#58;allocator<char> >) &#40;uci.cpp&#58;217&#41;
==2128==    by 0x10C059&#58; main &#40;main.cpp&#58;23&#41;

==2128== Conditional jump or move depends on uninitialised value&#40;s&#41;
==2128==    at 0x114318&#58; Engine&#58;&#58;negamax&#40;int, int, int, int&#41; &#91;clone .part.121&#93; &#40;hash.cpp&#58;356&#41;
==2128==    by 0x10F4F9&#58; Engine&#58;&#58;search_root&#40;) &#40;engine.cpp&#58;805&#41;
==2128==    by 0x11A312&#58; UCI&#58;&#58;go&#40;std&#58;&#58;__cxx11&#58;&#58;basic_istringstream<char, std&#58;&#58;char_traits<char>, std&#58;&#58;allocator<char> >&) &#40;engine.cpp&#58;965&#41;
==2128==    by 0x11267E&#58; UCI&#58;&#58;process&#40;std&#58;&#58;__cxx11&#58;&#58;basic_string<char, std&#58;&#58;char_traits<char>, std&#58;&#58;allocator<char> >) &#40;uci.cpp&#58;217&#41;
==2128==    by 0x10C059&#58; main &#40;main.cpp&#58;23&#41;

==2128== Conditional jump or move depends on uninitialised value&#40;s&#41;
==2128==    at 0x11B495&#58; Engine&#58;&#58;print_best_line&#40;int&#41; &#40;hash.cpp&#58;290&#41;
==2128==    by 0x115400&#58; Engine&#58;&#58;print_best_line&#40;int&#41; &#91;clone .constprop.9&#93; &#40;engine.cpp&#58;1027&#41;
==2128==    by 0x10F324&#58; Engine&#58;&#58;search_root&#40;) &#40;engine.cpp&#58;1132&#41;
==2128==    by 0x11A312&#58; UCI&#58;&#58;go&#40;std&#58;&#58;__cxx11&#58;&#58;basic_istringstream<char, std&#58;&#58;char_traits<char>, std&#58;&#58;allocator<char> >&) &#40;engine.cpp&#58;965&#41;
==2128==    by 0x11267E&#58; UCI&#58;&#58;process&#40;std&#58;&#58;__cxx11&#58;&#58;basic_string<char, std&#58;&#58;char_traits<char>, std&#58;&#58;allocator<char> >) &#40;uci.cpp&#58;217&#41;
==2128==    by 0x10C059&#58; main &#40;main.cpp&#58;23&#41;

To reduce the noise about the hash table a bit, I suggest the following patch:

Code: Select all

diff -r 3123b374dcc8 hash.cpp
--- a/hash.cpp	Thu Jun 01 03:34:40 2017 -0400
+++ b/hash.cpp	Thu Jun 01 12:37:28 2017 +0200
@@ -261,6 +261,7 @@
 TranspositionsTable::TranspositionsTable(size_t size_in_bytes) {
   size = size_in_bytes / sizeof(HashEntry);
   hash_table = new HashEntry[size];
+  memset(hash_table, 0x00, size);
 }

[AlvaroBegue]

Hi,

I suggest compiling it with the -ggdb3 flag and running it from valgrind.
This shows up quite a few uninitialized-but-used variables:

Code: Select all

[...]

To reduce the noise about the hash table a bit, I suggest the following patch:

Code: Select all

diff -r 3123b374dcc8 hash.cpp
--- a/hash.cpp	Thu Jun 01 03:34:40 2017 -0400
+++ b/hash.cpp	Thu Jun 01 12:37:28 2017 +0200
@@ -261,6 +261,7 @@
 TranspositionsTable::TranspositionsTable(size_t size_in_bytes) {
   size = size_in_bytes / sizeof(HashEntry);
   hash_table = new HashEntry[size];
+  memset(hash_table, 0x00, size);
 }

That's a great suggestion. Thanks!

NOTE: The memset is not quite right, because it clears `size' bytes, not `size' entries. But I can just call `TranspositionsTable::clear()' there. After that change valgrind only complains about the same things as it does if you run it on a hello world.

[mar]

Very nice, congratulations.

Maybe you should repost this notice in General forum as well
and I'm pretty sure you'd get a rating soon.

[jdart]

I use the MIT license (https://opensource.org/licenses/MIT).

--Jon

[Ras]

CppCheck has a couple of comments on the source code, maybe there are serious issues among them.

Since your make options allow for optimisations relying on strict pointer aliasing, it might be a good idea to test that with the GCC sanitiser options enabled. Works only under Linux.

[AlvaroBegue]

CppCheck has a couple of comments on the source code, maybe there are serious issues among them.

With --enable=warning it complains about several constructors that don't initialize things that don't need to be initialized.

Since your make options allow for optimisations relying on strict pointer aliasing, it might be a good idea to test that with the GCC sanitiser options enabled. Works only under Linux.

Oh, I didn't know about the -fsanitize options. I found an out-of-bounds array access in my SEE code. Thanks!

[Ras]

With --enable=warning it complains about several constructors that don't initialize things that don't need to be initialized.

engine.cpp also has several warnings for redundant condition checking:

Things like line 593

Code: Select all

(b.a[g2] == WhitePawn || b.a[g2] == WhitePawn)

might be an copy/paste issue because either the coordinate or the piece type could have been intended to actually be different.

Or also line 838

Code: Select all

if ((move_type == Quiet || move_type == DoublePawnPush || move_type == KingCastle || move_type == QueenCastle)

gets the annotation that the first term is always true. But if you had intended it to be that way, you would not have put a condition around the following block?


And yeah, the sanitiser is cool.