-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
add chess implementation #240
base: master
Are you sure you want to change the base?
Conversation
Glad you're doing this! Have you managed to train a decent chess bot with this? |
if gameResult == -1: | ||
if gameResult == 1: | ||
oneWon += 1 | ||
elif gameResult == 1: | ||
elif gameResult == -1: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seems to precisely invert the logic of which party won; was this done to fix a bug?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the original code is correct, this was not a bug.
Note a few lines up the order of the players was inverted, and also in Arena.py
it normalizes for player ID via curPlayer * self.game.getGameEnded(board, curPlayer)
here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This may be because the getGameEnded(self, board, player)
method of ChessGame returns 1 if player 1 won instead of the given player
won. Suggest multiplying the return value in ChessGame.getGameEnded
by the function parameter player
, instead of modifying the code here.
@@ -58,7 +58,7 @@ def executeEpisode(self): | |||
pi = self.mcts.getActionProb(canonicalBoard, temp=temp) | |||
sym = self.game.getSymmetries(canonicalBoard, pi) | |||
for b, p in sym: | |||
trainExamples.append([b, self.curPlayer, p, None]) | |||
trainExamples.append([self.game.toArray(b), self.curPlayer, p, None]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this would break Coach.py
for all other games
Note the same comment holds for the change in MCTS.py
if gameResult == -1: | ||
if gameResult == 1: | ||
oneWon += 1 | ||
elif gameResult == 1: | ||
elif gameResult == -1: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the original code is correct, this was not a bug.
Note a few lines up the order of the players was inverted, and also in Arena.py
it normalizes for player ID via curPlayer * self.game.getGameEnded(board, curPlayer)
here.
import sys | ||
sys.path.append('../../') | ||
from utils import * | ||
from pytorch_classification.utils import Bar, AverageMeter |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we can from utils import AverageMeter
, where is pytorch_classification.utils.Bar
though?
Resurrect PR Chess prototype #147 by @namin
self.play
stage, as i re-implement the functiongetGameEnded
Arena.py
where it showed some weird behaviour.stockfish
player.Once again, thanks @namin for the code implementation.