Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Analyse mode #266

Closed
wants to merge 6 commits into from
Closed

Analyse mode #266

wants to merge 6 commits into from

Conversation

WandererXII
Copy link
Contributor

@WandererXII WandererXII commented Jul 11, 2023

As discussed here: #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
Copy link
Contributor Author

WandererXII commented Jul 14, 2023

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 official-stockfish/Stockfish@ba4e215#diff-1d066214056be2b8958a2a2143c121d339937f753cdd7b5361cf1cece354480dR1077

But I'm not familiar enough with the codebase to be sure. It works for the cases described in #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
Copy link
Owner

I fixed this issue with this : d49b90b
I apologize for making this pull request unnecessarily. Thank you for the detailed report .

@yaneurao yaneurao closed this Oct 21, 2023
@WandererXII
Copy link
Contributor Author

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
Copy link
Contributor Author

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
Copy link
Owner

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
Copy link
Owner

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.

aac8736

@WandererXII
Copy link
Contributor Author

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants