Stockfish 1.8 - eval cache

[zamar]

Again a little something from search.cpp.

In root_search() you compute

depth = (Iteration - 2) * OnePly + InitialDepth;

nested within the fail low and the move loop.

Depth computation could be done in Step 1. Initialize node.
No reason to compute it within any loops.

root_search() is not time critical code, so for speed it doesn't matter at all.

Now the question is that does your change increase the readability of the code? What do you think yourself?

[mcostalba]

No reason to compute it within any loops.

You are right. Thanks for noticing and reporting.

Ooops I see now the comment of Joona, well IMHO we could push the patch:

Code: Select all

diff --git a/src/search.cpp b/src/search.cpp
index 2ab9839..fff0002 100644
--- a/src/search.cpp
+++ b/src/search.cpp
@@ -715,6 +715,7 @@ namespace {
     alpha = *alphaPtr;
     beta = *betaPtr;
     isCheck = pos.is_check();
+    depth = (Iteration - 2) * OnePly + InitialDepth;
 
     // Step 1. Initialize node (polling is omitted at root)
     ss->currentMove = ss->bestMove = MOVE_NONE;
@@ -764,7 +765,6 @@ namespace {
             captureOrPromotion = pos.move_is_capture_or_promotion(move);
 
             // Step 11. Decide the new search depth
-            depth = (Iteration - 2) * OnePly + InitialDepth;
             ext = extension<PV>&#40;pos, move, captureOrPromotion, moveIsCheck, false, false, &dangerous&#41;;
             newDepth = depth + ext;
 

What do you think ? I have moved to variable initializations, not under step 1 to let step 1 be more in line with corresponding parts in the searches.

[Ralph Stoesser]

Again a little something from search.cpp.

In root_search() you compute

depth = (Iteration - 2) * OnePly + InitialDepth;

nested within the fail low and the move loop.

Depth computation could be done in Step 1. Initialize node.
No reason to compute it within any loops.

root_search() is not time critical code, so for speed it doesn't matter at all.

Now the question is that does your change increase the readability of the code? What do you think yourself?

There is a little stylistically point in doing it "en block", near the extension calculation, but finally it's clear initialization code. Therefore to me it looks a little wrong, also with respect to readability. But for sure it's a (very) minor issue and I thought a while about whether it's worth to report or not.

What's your opinion?

@Marco, sure, variable initialization part would be ok.

[mcostalba]

I thought a while about whether it's worth to report or not.

What's your opinion?

I like this kind of reports, perhaps I am a bit weird but fixing these details for me is like fixing the big issues

[Ralph Stoesser]

I thought a while about whether it's worth to report or not.

What's your opinion?

I like this kind of reports, perhaps I am a bit weird but fixing these details for me is like fixing the big issues

Good to know.
It's not that I would search for such things. They cross my path while I try changes which do not work.

[Ralph Stoesser]

Ok, now something for the real nitpicking fans.

You have such wonderfull constants like
MOVE_NONE, VALUE_NONE, VALUE_TYPE_NONE, SQ_NONE, FILE_NONE, RANK_NONE, DIR_NONE, COLOR_NONE, SCALE_FACTOR_NONE, DEPTH_NONE (recently added)

but also a few horribles like
NO_PIECE, NO_PIECE_TYPE, NO_CASTLES

I'm accustomed to the *_NONE scheme, so I propose to change the NO_* constants to *_NONE's.

Btw: I use Eclipse as IDE.
There is a wonderfull function "refactor/rename..."

[mcostalba]

Btw: I use Eclipse as IDE.
There is a wonderfull function "refactor/rename..."

I used QtCreator, last version 2.0 is really awsome for C++ ! Parses better then MSVC IDE...and of course has renames too

BTW while doing your proposed renaming I quickly found a couple of other spots:

Code: Select all

Position::captured_piece()   ->  Position::captured_piece_type()

st->capture   ->  st->capturedType

I think proper variabe naming it is a great help in code readibility and is far from trivial as someone (with not much programming experience) could think.

[Ralph Stoesser]

Hi,

Is SF 1.8x now bug free, with maximum beautiful code?
If so, why don't you release it?

falling asleep greetings ...

[mcostalba]

Hi,

Is SF 1.8x now bug free, with maximum beautiful code?
If so, why don't you release it?

falling asleep greetings ...

Code could be nice...but we need some real ELO increase before to release

...and it seems harder and harder to get...

[Uri Blass]

Hi,

Is SF 1.8x now bug free, with maximum beautiful code?
If so, why don't you release it?

falling asleep greetings ...

Code could be nice...but we need some real ELO increase before to release

...and it seems harder and harder to get...

There is no rule that forbid releasing a new version with no elo increase.

Note that it is surprising to read that it is harder to get elo improvement considering the fact that stockfish evaluates every node in the tree and it should make improvement relatively easier(because you can try using evaluation better for decision if to prune or not to prune).

Considering the fact that stockfish tends to outsearch everything including rybka(even after we add some plies to the search of rybka) I think that you probably can improve stockfish by having some rules to do less pruning in the right positions of the tree(because I guess that a relative advantage of rybka is that she knows when not to prune) and I wondered if you tried some rules to limit pruning.