-
Notifications
You must be signed in to change notification settings - Fork 6
Conversation
floo51
commented
Sep 22, 2016
•
edited
edited
- Make it work
- Factorize & test
Ready for review |
[True, False, False, False], | ||
[False, True, False, False], | ||
[False, False, True, False], | ||
[False, False, False, True] |
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.
Put a comma at each end of line
|
||
|
||
class TestStringMethods(unittest.TestCase): | ||
|
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.
Remove this line
mask = tf.placeholder('bool', [4, 4]) | ||
|
||
|
||
def is_aligned(): |
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.
You can transform this from a function to a variable like this:
extract_chunk = tf.boolean_mask(board, mask)
is_aligned = tf.equal(tf.reduce_prod(extract_chunk), 1)
In this way:
- you avoid to recreate variables each time you use this function
- you save RAM
- you can import
extract_chunk
from this module too
import unittest | ||
import tensorflow as tf | ||
|
||
from reward import * |
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.
Consider import *
is forbidden. Import each function at a time.
from reward import is_aligned
or use it directly from the module:
import reward
# Usage
reward.is_aligned
] | ||
|
||
self.assertEqual( | ||
session.run(is_aligned(), feed_dict={ |
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.
For readability purpose, put the session run result in a variable, then check it like:
this.assertEqual(result, True)
This will help you give name to things too.
@@ -0,0 +1,19 @@ | |||
import tensorflow as tf | |||
|
|||
from ai.bitmasks import * |
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.
Forbidden import *
Most of |
You don't have Here is a Dockerfile that you can use for your project:
And the related Makefile commands: install:
docker build -t connectfour .
run:
docker run --rm --it -v $(shell pwd):/code connectfour python connectfour.py
test:
docker run --rm --it -v $(shell pwd):/code connectfour python -m unittest discover You may change some things but the most of it is here. |
Review applied |