Designing an analysis friendly Stockfish?

[Jeremy Bernstein]

Here's the git repository with the most recent source changes, including those suggested by BB+ this morning:

https://github.com/jeremybernstein/Stockfish_PA_GTB

SORRY, I screwed up some of the logic in search.cpp and broke some things, rebuilding now.

[BB+]

There was a question about tbhits not appearing. This might be a problem with gathering data from splitpoints in SMP mode. The "nodes_searched" variable has the same difficulty.

[Ted Summers]

There was a question about tbhits not appearing. This might be a problem with gathering data from splitpoints in SMP mode. The "nodes_searched" variable has the same difficulty.

Yes, I deleted it after I saw ...

"SORRY, I screwed up some of the logic in search.cpp and broke some things, rebuilding now." from Jeremy

[fruity]

SF PA_G scores about 15 Elo below SF default at very fast games in my tests. If everything would have been correctly coded in SF PA_G that should not be the case. I think the ok_to_use_tt_PV() function is not quite right. The test for TT hits should rather look like

Code: Select all

bool ok_to_use_TT(const TTEntry* tte, Depth depth, Value alpha, Value beta, int ply) {

    Value v = value_from_tt(tte->value(), ply);

    return (   tte->depth() >= depth
            || v >= Max(value_mate_in(PLY_MAX), beta)
            || v <= Min(value_mated_in(PLY_MAX), alpha))

           && (
        		    ((tte->type() & VALUE_TYPE_LOWER) && v >= beta)
        		 || ((tte->type() & VALUE_TYPE_UPPER) && v <= alpha)
        		 || (tte->type() == VALUE_TYPE_EXACT && v < beta && v > alpha)
        	  );
  }

in both cases, PV and non-PV. Please correct me if I'm wrong. If I change the code appropriately, I get a slightly positive score after 6000 ultra fast games, like expected. Result was: +1921 -1814 =2265 from changed SF PA_G's pov.

I had not touched ok_to_use_TT but I will go ahead and fix this. Was the +1921 -1814 =2265 score from the 20" games?

http://dl.dropbox.com/u/11904592/stockfish_201_PA_I.zip

Here is a new build with the proposed corrections.

I only changed this for non-PV. Should I really change this for ok_to_use_TT_PV as well? I though the behavior should be fundamentally different for PV entries.

No, score was from 6000 1'' games. 20'' games are still running.

I'm not sure whether it's good to use my changed ok_to_use_TT() in PV and non-PV case, but for sure it makes not much sense to have the identical function twice in search.cpp.

In the original ok_to_use_TT_PV() there is a test for tte->move() != MOVE_NONE. Is this test necessary? This is not critical for the Elo performance, I just wonder why it was there...

[Jeremy Bernstein]

There was a question about tbhits not appearing. This might be a problem with gathering data from splitpoints in SMP mode. The "nodes_searched" variable has the same difficulty.

Yes, I deleted it after I saw ...

"SORRY, I screwed up some of the logic in search.cpp and broke some things, rebuilding now." from Jeremy

Argh. In my defense, it looks like the problem is somewhere in the new GTB code. I've applied the PA_I changes + BB+s fixes to the old GTB code and it's working again, although I'm getting an assertion in update_history() that I'm looking into now. Sorry for the inconvenience. I hope to have something up again shortly.

[Jeremy Bernstein]

Two questions: BB+, does this

Code: Select all

    
    // When using Standard C input functions, also check if there
    // is anything in the buffer. After a call to such functions,
    // the input waiting in the pipe will be copied to the buffer,
    // and the call to PeekNamedPipe can indicate no input available.
    // Setting stdin to unbuffered was not enough. [from Greko]
    if (stdin->_cnt > 0)
        return 1;

belong at the top or the bottom of the function?

Now, the assertion I'm getting is the result of this code:

Code: Select all

        if (    (bestValue >= beta || vt == VALUE_TYPE_EXACT)
            && !pos.move_is_capture_or_promotion(move))
        {
            update_history(pos, move, depth, movesSearched, moveCount);
            update_killers(move, ss);
        }

Reverting it to the previous version (no test for vt == VALUE_TYPE_EXACT) eliminates the assertion (where the move passed into to update_history() is in the MoveSearched array). I don't know if this assertion is actually protecting us from something bad, though.

Any ideas?

Thanks
Jeremy

[BB+]

Not sure about where that goes. GreKo (which is from where Marco took it) has it at the top:

Code: Select all

int InputAvailable()
{
        DWORD nchars;

        /* When using Standard C input functions, also check if there
        is anything in the buffer. After a call to such functions,
        the input waiting in the pipe will be copied to the buffer,
        and the call to PeekNamedPipe can indicate no input available.
        Setting stdin to unbuffered was not enough, IIRC */

        if (stdin->_cnt > 0)
                return 1;

        if (is_pipe)
        {
                /* When running under a GUI, you will end here. */

                if (!PeekNamedPipe(input_handle, NULL, 0, NULL, &nchars, NULL))

                        /* Something went wrong. Probably the parent program exited.
                                Could call exit() here. Returning 1 will make the next call
                                to the input function return EOF, where this should be
                                catched then. */

                        return 1;

                return (nchars != 0);
        }
        else
                return _kbhit() != 0; /* In "text-mode" without GUI */

Fruit 2.1 also has it first:

Code: Select all

bool input_available() {

#if defined(_WIN32) || defined(_WIN64)

   static bool init = false, is_pipe;
   static HANDLE stdin_h;
   DWORD val, error;

   // val = 0; // needed to make the compiler happy?

   // have a look at the "local" buffer first, *this time before init (no idea if it helps)*

   if (UseDebug && !init) printf("info string init=%d stdin->_cnt=%d\n",int(init),stdin->_cnt);

   if (stdin->_cnt > 0) return true; // HACK: assumes FILE internals

   // input init (only done once)

   if (!init) {

      init = true;

      stdin_h = GetStdHandle(STD_INPUT_HANDLE);

      if (UseDebug && (stdin_h == NULL || stdin_h == INVALID_HANDLE_VALUE)) {
         error = GetLastError();
         printf("info string GetStdHandle() failed, error=%d\n",error);
      }

      is_pipe = !GetConsoleMode(stdin_h,&val); // HACK: assumes pipe on failure

      if (UseDebug) printf("info string init=%d is_pipe=%d\n",int(init),int(is_pipe));

      if (UseDebug && is_pipe) { // GetConsoleMode() failed, everybody assumes pipe then
         error = GetLastError();
         printf("info string GetConsoleMode() failed, error=%d\n",error);
      }

      if (!is_pipe) {
         SetConsoleMode(stdin_h,val&~(ENABLE_MOUSE_INPUT|ENABLE_WINDOW_INPUT));
         FlushConsoleInputBuffer(stdin_h); // no idea if we can lose data doing this
      }
   }

   if (is_pipe) {

      if (!PeekNamedPipe(stdin_h,NULL,0,NULL,&val,NULL)) {

         if (UseDebug) { // PeekNamedPipe() failed, everybody assumes EOF then
            error = GetLastError();
            printf("info string PeekNamedPipe() failed, error=%d\n",error);
         }

         return true; // HACK: assumes EOF on failure
      }

      if (UseDebug && val < 0) printf("info string PeekNamedPipe(): val=%d\n",val);

      return val > 0; // != 0???

   } else {

      GetNumberOfConsoleInputEvents(stdin_h,&val);
      return val > 1; // no idea why 1
   }

   return false;

[BB+]

So you are saying that the assert fails in this?

Code: Select all

void update_history(const Position& pos, Move move, Depth depth,
                     Move movesSearched[], int moveCount)
{
    Move m;
    H.success(pos.piece_on(move_from(move)), move_to(move), depth);
    for (int i = 0; i < moveCount - 1; i++)
    {
        m = movesSearched[i];
        assert(m != move); // FAILURE ?
        if (!pos.move_is_capture_or_promotion(m))
            H.failure(pos.piece_on(move_from(m)), move_to(m), depth); } }

[Jeremy Bernstein]

There was a question about tbhits not appearing. This might be a problem with gathering data from splitpoints in SMP mode. The "nodes_searched" variable has the same difficulty.

Yes, I deleted it after I saw ...

"SORRY, I screwed up some of the logic in search.cpp and broke some things, rebuilding now." from Jeremy

Argh. In my defense, it looks like the problem is somewhere in the new GTB code. I've applied the PA_I changes + BB+s fixes to the old GTB code and it's working again, although I'm getting an assertion in update_history() that I'm looking into now. Sorry for the inconvenience. I hope to have something up again shortly.

Here are new builds, based on the PA_I code and previous GTB code. GTB probing works again. I removed the code causing the update_history() assertion pending further investigation. The code on github reflects these builds.

[Jeremy Bernstein]

So you are saying that the assert fails in this?

Code: Select all

void update_history(const Position& pos, Move move, Depth depth,
                     Move movesSearched[], int moveCount)
{
    Move m;
    H.success(pos.piece_on(move_from(move)), move_to(move), depth);
    for (int i = 0; i < moveCount - 1; i++)
    {
        m = movesSearched[i];
        assert(m != move); // FAILURE ?
        if (!pos.move_is_capture_or_promotion(m))
            H.failure(pos.piece_on(move_from(m)), move_to(m), depth); } }

Exactly.