yaneurao / YaneuraOu

YaneuraOu is the World's Strongest Shogi engine(AI player) , WCSC29 1st winner , educational and USI compliant engine.
GNU General Public License v3.0
512 stars 140 forks source link

Analyse mode #266

Closed WandererXII closed 11 months ago

WandererXII commented 1 year ago

As discussed here: https://github.com/yaneurao/YaneuraOu/issues/264

This is trying to solve two problems:

  1. Currently repetitions are marked after the first repeated position. This is fine for playing, but not for analyzing, because it leads to graphs like this: image Here's another simple example, in this example, I first captured opponent's bishop and started repeating the positions. With red I marked the repeated positions, and in orange I marked the incorrect evaluations. yanerr_REP_P After this patch with USI_AnalyseMode: yancor_REP

  2. Superior position returning 31111 cp is a problem for analyzing because of graphs like this: image

I try to solve this by adding is_repetition_full function that checks for all 4 repetitions (still only to rep_ply depth) before or at the root and checks for superior positions only after root. is_repetition_full is called in yaneuraou-search if USI_AnalyseMode is true. No functional change if USI_AnalyseMode is false.

Related commit/PR from Stockfish:

WandererXII commented 1 year ago

Not entirely sure about the ply > i in is_repetition_full. The point is to limit the search after the root, so it doesn't go into repeated positions 3 times, but still check for it before or at the root (and the same thing with superior positions), basically what the stockfish comment says:

// Return a draw score if a position repeats once earlier but strictly
// after the root, or repeats twice before or at the root.

For shogi it would be:

// Return a draw score if a position repeats once earlier but strictly
// after the root, or repeats three times before or at the root.

I took the ply > i from https://github.com/official-stockfish/Stockfish/commit/ba4e215493de31263b9bd352af79d00193e545bf#diff-1d066214056be2b8958a2a2143c121d339937f753cdd7b5361cf1cece354480dR1077

But I'm not familiar enough with the codebase to be sure. It works for the cases described in https://github.com/yaneurao/YaneuraOu/issues/264, but there might be some edge cases...

Also maybe worth considering making this the default behavior for is_repetition and not adding another option - USI_AnalyseMode?

yaneurao commented 11 months ago

I fixed this issue with this : https://github.com/yaneurao/YaneuraOu/commit/d49b90b23ff70ac93f7031d40170ff9eb50f8313 I apologize for making this pull request unnecessarily. Thank you for the detailed report .

WandererXII commented 10 months ago

Thanks for working on this. But the repetitions before the 4th one still show zero.

image

This still should not be draw. position sfen lnsgkgsnl/1r5b1/ppppppppp/9/9/9/PPPPPPPPP/1B5R1/LNSGKGSNL b - 1 moves 7g7f 3c3d 8h2b+ 8b7b 2b8h 7b8b 8h7g 8b7b 7g8h 7b8b 8h7g 8b7b 7g8h 7b8b 8h7g

WandererXII commented 10 months ago

Would you accept wrapping the draw in this if statement?

if (++cnt + (i < sup_rep_ply ? 2 : 0) >= 3)

Therefore something like this:

// is_repetition()の、千日手が見つかった時に、現局面から何手遡ったかを返すバージョン。
// found_plyにその値が返ってくる。
RepetitionState Position::is_repetition2(int repPly, int sup_rep_ply) const
{
    // pliesFromNullが未初期化になっていないかのチェックのためのassert
    ASSERT_LV3(st->pliesFromNull >= 0);

    // 遡り可能な手数。
    // 最大でもrepPly手までしか遡らないことにする。
    int end = std::min(repPly, st->pliesFromNull);

    // 少なくとも4手かけないと千日手にはならないから、4手前から調べていく。
    if (end < 4)
        return REPETITION_NONE;

    int cnt = 0;
    StateInfo* stp = st->previous->previous;

    for (int i = 4; i <= end ; i += 2)
    {
        stp = stp->previous->previous;

        // board_key : 盤上の駒のみのhash(手駒を除く)
        // 盤上の駒が同じ状態であるかを判定する。
        if (stp->board_key() == st->board_key())
        {
            // 手駒が一致するなら同一局面である。(2手ずつ遡っているので手番は同じである)
            if (stp->hand == st->hand)
            {
                if (++cnt + (i < sup_rep_ply ? 2 : 0) >= 3)
                {
                    // 自分が王手をしている連続王手の千日手なのか?
                    if (i <= st->continuousCheck[sideToMove])
                        return REPETITION_LOSE;

                    // 相手が王手をしている連続王手の千日手なのか?
                    if (i <= st->continuousCheck[~sideToMove])
                        return REPETITION_WIN;

                    return REPETITION_DRAW;
                }
                else {
                    end = std::min(end + repPly, st->pliesFromNull);
                }
            }
            else if (i < sup_rep_ply) {
                    // ↑rootより遡らないための措置

                // 優等局面か劣等局面であるか。(手番が相手番になっている場合はいま考えない)
                if (hand_is_equal_or_superior(st->hand, stp->hand))
                    return REPETITION_SUPERIOR;
                if (hand_is_equal_or_superior(stp->hand, st->hand))
                    return REPETITION_INFERIOR;
            }
        }
    }

    // 同じhash keyの局面が見つからなかったので…。
    return REPETITION_NONE;
}

It's fine if this is not something you want to do, I can use my fork for my needs. Thanks!

yaneurao commented 10 months ago

When attempting to analyze a shogi game record played by a human, it's understandable that it's not preferable to consider it a draw by repetition draw at the second occurrence of the same position.

However, due to this modification, there is a slight decrease Elo, so I will think of a better solution.

yaneurao commented 10 months ago

This issue has been resolved in the following commit. I have modified the code to not return the repetition draw evaluation value (-1) until the fourth occurrence of the same position. Also, there is almost no decrease in Elo due to this fix. I apologize for the long wait.

https://github.com/yaneurao/YaneuraOu/commit/aac87364db470dec505c1138d50508d322fb3fdd

WandererXII commented 10 months ago

Great work! Really happy to see this. Thanks a lot!