Skip to content
This repository has been archived by the owner on Apr 17, 2019. It is now read-only.

[RFR] Get board reward #2

Merged
merged 12 commits into from
Oct 7, 2016
Merged

[RFR] Get board reward #2

merged 12 commits into from
Oct 7, 2016

Conversation

floo51
Copy link
Contributor

@floo51 floo51 commented Sep 22, 2016

⚠️ Waiting for merge + rebase : do not review

  • Get board reward
  • Add tests

@floo51 floo51 changed the title [WIP] Get board reward [RFR] Get board reward Sep 22, 2016
@floo51
Copy link
Contributor Author

floo51 commented Sep 22, 2016

Rebased, ready for review

player_has_consecutive_chunks = tf.equal(consecutive_chunks, 1)
is_game_won = tf.reduce_any(player_has_consecutive_chunks)

get_board_reward = tf.case(
Copy link
Contributor

Choose a reason for hiding this comment

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

Make it more readable:

get_board_reward = tf.case(
    {is_game_won: reward_positive, is_game_lost: reward_negative},
    default=reward_none,
    exclusive=True
)

print session.run(is_aligned, feed_dict={
board: game_board,
mask: bitmasks[2]
print session.run(get_board_reward, feed_dict={
Copy link
Contributor

Choose a reason for hiding this comment

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

In python 3, print is not a built-in keyword anymore but a function.
Please use it as a function to avoid error when you'll switch to python 3.

@Kmaschta
Copy link
Contributor

Can you hook unit tests with Travis please?

@floo51
Copy link
Contributor Author

floo51 commented Oct 3, 2016

Last version pushed, as per demo.
Previous comments taken into account, ready for new review.

@floo51 floo51 mentioned this pull request Oct 3, 2016
Copy link
Contributor

@Kmaschta Kmaschta left a comment

Choose a reason for hiding this comment

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

Please use one of these format instead of string concatenations:

>>> a, b  = 5, 10
>>> 'some {} thing {}'.format(a, b)
"some 5 thing 10"
>>> 'foo %s bar %s' % (a, b)
"foo 5 bar 10"

Great work!


def turn_feedback(self, player, column):
who = 'You'
if self.player != player:
Copy link
Contributor

Choose a reason for hiding this comment

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

who = 'You' if self.playeur == playeur else 'Opponent'

game_copy.play(i, current_player)
legal_moves.append(game_copy)

if depth == 0 or len(legal_moves) == 0 or game.get_status() == GAME_STATUS['FINISHED']:
Copy link
Contributor

Choose a reason for hiding this comment

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

Give a name to your conditions in order to make it more readable. For example:

first_turn = depth == 0
no_legal_moves = len(legal_moves) == 0
game_finished = game.get_status() == GAME_STATUS['FINISHED']

if first_turn or no_legal_moves or game_finished:
    pass # Do the flop


if opp_fours > 0:
return -100000
else:
Copy link
Contributor

Choose a reason for hiding this comment

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

Useless else statement

@@ -0,0 +1,5 @@
# import unittest
Copy link
Contributor

Choose a reason for hiding this comment

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

Commented out code

from tool.array_transposer import transpose_horizontally, transpose_diagonally


GAME_STATUS = {
Copy link
Contributor

Choose a reason for hiding this comment

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

Not important for this review, but the good practice in python is to define constants list as:

GAME_PLAYING = 0
GAME_FINISHED = 1
GAME_STATUS = (
    ('PLAYING', GAME_PLAYING),
    ('FINISHED', GAME_FINISHED),
)

elif player2_won:
self.winner = 1

status = GAME_STATUS['FINISHED'] \
Copy link
Contributor

Choose a reason for hiding this comment

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

Where the ternary condition is too long, it's better to use the classical if/else condition

aligned_discs = self.has_aligned_discs(number_of_cells)
if player in aligned_discs:
return aligned_discs[player]
else:
Copy link
Contributor

Choose a reason for hiding this comment

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

Useless else

@fzaninotto fzaninotto merged commit 9a6f427 into master Oct 7, 2016
@fzaninotto fzaninotto deleted the get_board_reward branch October 7, 2016 12:38
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants