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

rbc ActionToString sometimes returns ambiguous moves in SAN #697

Closed
VitamintK opened this issue Aug 25, 2021 · 3 comments
Closed

rbc ActionToString sometimes returns ambiguous moves in SAN #697

VitamintK opened this issue Aug 25, 2021 · 3 comments

Comments

@VitamintK
Copy link
Contributor

VitamintK commented Aug 25, 2021

example:

game = pyspiel.load_game('rbc', {'fen': '2kr1n1N/pp2p3/8/2P2pp1/2P1p3/4B3/r1PK1P2/8 b - - 1 25'})
state = game.new_initial_state().child(0)
print([state.action_to_string(a) for a in state.legal_actions()])

results include both Raxd2 and Rxd2, but should instead include Raxd2 and Rdxd2, since Rxd2 is ambiguous.

(the board is

. . k r . n . N
p p . . p . . .
. . . . . . . .
. . P . . p p .
. . P . p . . .
. . . . B . . .
r . P K . P . .
. . . . . . . .

)

This is due to ChessBoard's Move.ToSan() using only (pseudo)-legal moves to disambiguate, of which Raxd2 is not one, so it thinks disambiguation is not necessary.

Sorry to raise yet another issue for reconchess! 😛 I can make the fix for this one, but I'm not sure how to do it: pass an extra argument to ToSAN that's a vector of all moves to disambiguate between?

Or is the better fix to just switch ActionToString to use ToLAN instead? This might be the better choice, since some SAN concepts fall apart in recon chess anyways: e.g. an action could be either Qd2 or Qxd2, but with LAN the action only has one valid representation.

@lanctot
Copy link
Collaborator

lanctot commented Aug 25, 2021

Sorry to raise yet another issue for reconchess

Don't be sorry, this is what makes open source work so well.

But do please tag @michalsustr on any issues primarily about RBC as he is the primary author and is maintaining it. Thanks!

@michalsustr
Copy link
Collaborator

Yes ToLAN is a better choice as you explained. Can you post a PR?

@lanctot
Copy link
Collaborator

lanctot commented Sep 20, 2021

I believe this was fixed by #704. If there are any outstanding problems on this issue, please re-open.

@lanctot lanctot closed this as completed Sep 20, 2021
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

No branches or pull requests

3 participants