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

[Shogi] Add and fix buggy test samples #358

Merged
merged 12 commits into from
Feb 19, 2023
Merged

[Shogi] Add and fix buggy test samples #358

merged 12 commits into from
Feb 19, 2023

Conversation

youyou-ku
Copy link
Collaborator

cshogiと挙動が違う盤面を探す。とりあえずは挙動が変わってしまいそうな(両王手など)盤面をいくつかサンプルで入れて比較、みたいなことをする
cshogiでランダムな盤面を作って、それをStateに変換し、それぞれのlegal_action_maskを比較、みたいなことも必要か

検証用コード
def f(sfen):
    c_board = cshogi.Board(sfen)
    p_board = _sfen_to_state(sfen)
    turn = int(p_board.turn)
    c_legal_actions = jnp.zeros(27 * 81, dtype=jnp.int8)
    for move in c_board.legal_moves:
        ind = make_move_label(move, turn)
        c_legal_actions = c_legal_actions.at[ind].set(1)
    p_legal_actions = _legal_actions(p_board)
    p_legal_actions = _to_direction(p_legal_actions)
    if jnp.all(c_legal_actions == p_legal_actions):
        return
    for i in range(27*81):
        if c_legal_actions[i] != p_legal_actions[i]:
            print(i)


if __name__ == "__main__":
    f("8k/9/9/8l/9/9/7n1/9/8K b 2r2b4g4s3n3l18p 1")

@youyou-ku
Copy link
Collaborator Author

youyou-ku commented Feb 16, 2023

image
sfen: l+B6l/6k2/3pg2P1/p6p1/1pP1pB2p/2p3n2/P+r1GP3P/4KS1+s1/LNG5L b RGN2sn6p 1
1125 1188 1279 1369 1424 1432 と駒打ちのところが不一致。
それぞれ左成72 右成54 下成64…という感じで、馬の動作と一致。馬の移動は成不可だがpgxでは生成されてしまっている
駒打ちのところは、もしかしたらhandの配列が違うので対応するdirectionも違うかも

@youyou-ku
Copy link
Collaborator Author

youyou-ku commented Feb 16, 2023

駒打ちを初期盤面に持ち駒を加えて確認したが、そもそも歩以外の駒の駒打ちが生成できていないっぽい?
→駒全種持たせたらちゃんと生成した。単独だとなぜか生成しない
歩だけなぜか(二歩にならないように盤面の歩を消しても)生成されてた
legal_action_maskの順番については、角打ちが24、飛車打ちが25、金打ちが26だったのでhandの仕様の違いは関係ない

@youyou-ku
Copy link
Collaborator Author

youyou-ku commented Feb 16, 2023

→駒全種持たせたらちゃんと生成した。単独だとなぜか生成しない

初期盤面での検証だが、駒を複数(同じもの二枚でも)持っている場合はちゃんと生成される。一種一枚の時だけ生成されない(歩はいけるけど)
ただ発見したときの状況だと桂金飛一枚ずつ持っているので上の条件には当てはまらない。盤面依存で生成されないときがある?もしくはあいての持ち駒による影響?

@youyou-ku
Copy link
Collaborator Author

youyou-ku commented Feb 16, 2023

飛車とかの駒打ちの時にも二歩条件がかかっているっぽい。自分の歩が存在するラインにあらゆる駒を置けなくなっている
上の条件で複数駒にしたらできたのは、単にその前に歩打ちの判定のために歩を全部取っ払っていたからというだけみたい

@youyou-ku
Copy link
Collaborator Author

youyou-ku commented Feb 16, 2023

とりあえずテストケースとして

  • 成れない駒(成駒、金、玉)の成移動チェック
  • 歩以外の持ち駒に対しての二歩判定のチェック

を追加してマージします。

@sotetsuk
Copy link
Owner

@youyou-ku マージはちょっとまってもらっていいですか?テストケース追加したらテスト落ちますよね

Comment on lines 384 to 388
#c_action = jnp.zeros(27 * 81, dtype=jnp.int8)
#c_action = c_action.at[1701:2187].set(1)
#c_action = c_action.at[jnp.arange(81 * 21 + 6, 81 * 27, 9)].set(1)
#c_action = c_action.at[jnp.arange(5, 81, 9)].set(1)
#assert (legal_action == c_action).all
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ここよくわからないです。
多分べつに all で判定しなくても、いいと思うんですけど
テストももう少しコメントしたりわかりやすくしてほしいです

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

座標していはなるべく xy2i を使ってください

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@youyou-ku visualize使って画像も追加してください(他のテスト参考にしてください)

@sotetsuk
Copy link
Owner

image

1125 1188 1279 1369 1424 1432 と駒打ちのところが不一致。 それぞれ左成72 右成54 下成64…という感じで、馬の動作と一致。馬の移動は成不可だがpgxでは生成されてしまっている 駒打ちのところは、もしかしたらhandの配列が違うので対応するdirectionも違うかも

これのsfenってあります?
なるべくコメントは画像だけは避けてほしいです。
E.g., スクショだけ 🙅‍♀️ コード 🙆‍♀️

@youyou-ku
Copy link
Collaborator Author

l+B6l/6k2/3pg2P1/p6p1/1pP1pB2p/2p3n2/P+r1GP3P/4KS1+s1/LNG5L b RGN2sn6p 1
です🙇

Comment on lines 396 to 401
# 成駒のpromotion判定
sfen = "9/2+B1G1+P2/9/9/9/9/9/9/9 b - 1"
legal_action = _to_direction(_legal_actions(_sfen_to_state(sfen)))
legal_action = jnp.where(legal_action, 1, 0)
# promotionは生成されてたらダメ
assert jnp.all(legal_action[810:] == 0)
Copy link
Owner

@sotetsuk sotetsuk Feb 19, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

このテスト意味がわからないんですけどどういう意味ですか?
legal_action = jnp.where(legal_action, 1, 0) で全部0になってますよね?

@youyou-ku

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

とりあえずコメントを参考に修正済

# 三列目には打てる
assert (legal_action[81*22+1:81*23:9] == False).all()
assert (legal_action[81*22:81*23:9] == False).all()
assert (legal_action[81*21+5:81*22:9] == False).all()
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

== False みたいなコードを書くのはやめましょう

(~legal_action[81*21+5:81*22:9]).all()

@sotetsuk sotetsuk changed the title [Shogi] compare with cshogi [Shogi] Add buggy test samples Feb 19, 2023
@sotetsuk sotetsuk changed the title [Shogi] Add buggy test samples [Shogi] Add and fix buggy test samples Feb 19, 2023
@sotetsuk sotetsuk merged commit 05d6cd8 into main Feb 19, 2023
@sotetsuk sotetsuk deleted the dif_from_cshogi branch February 19, 2023 16:50
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