-
Notifications
You must be signed in to change notification settings - Fork 10
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
Dev basis sentiment infer 20190122 #4
base: master
Are you sure you want to change the base?
Conversation
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.
Mostly minor comments. Only additional questions I have is:
- Have you tested that all prior endpoints still work (for images)
- Could you add an updated README to your PR that describes how to use the sentiment API?
@@ -110,48 +112,55 @@ def transfer_model(self, local_dir, | |||
transfer the topless InceptionV3 model | |||
to classify new classes | |||
""" | |||
print "Inside Transfer model" |
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 don't think we need this. Seems like an artifact of your debugging :)
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.
yes, I will remove this and other debugging related print statements
# set up parameters | ||
nb_train_samples = self.__get_nb_files(train_dir) | ||
nb_classes = len(glob.glob(train_dir + "/*")) | ||
nb_val_samples = self.__get_nb_files(val_dir) | ||
nb_epoch = int(nb_epoch) | ||
batch_size = int(batch_size) | ||
|
||
|
||
print "nb_val_samples:{}".format(nb_val_samples) |
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 don't think we need this either, especially if we want to move to Python 3 down the line
def __get_nb_files(self, directory): | ||
"""Get number of files by searching local dir recursively""" | ||
logging.info("Inside __get_nb_files") |
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.
Same as above
|
||
|
||
if textIDs: | ||
print("* Predicting for {} of Models".format(len(textIDs.keys()))) |
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.
We should either use print everywhere or logging everywhere (I vote logging)
print("* Predicting for {} of Models".format(len(textIDs.keys()))) | ||
print("* Number of Sentences: {}".format(num_text)) | ||
|
||
r = {"positive":0.5, "negative":0.5} |
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.
What is r
for?
@@ -19,6 +19,9 @@ | |||
INV3_TRANSFER_BATCH_SIZE = app.config['INV3_TRANSFER_BATCH_SIZE'] | |||
INCEPTIONV3_IMAGE_QUEUE = app.config['INCEPTIONV3_IMAGE_QUEUE'] | |||
INCEPTIONV3_TOPLESS_MODEL_PATH = app.config['INCEPTIONV3_TOPLESS_MODEL_PATH'] | |||
|
|||
SENTIMENT_TEXT_QUEUE = app.config['SENTIMENT_TEXT_QUEUE'] #Added by MS on 22-Jan-2019 |
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 don't think you need that comment
image_data_path = API_helpers.download_a_dir_from_s3(bucket_name = s3_bucket_name, | ||
bucket_prefix = s3_bucket_prefix, | ||
local_path = TEMP_FOLDER) | ||
try: | ||
# init the transfer learning manager | ||
this_IV3_transfer = inceptionV3_transfer_retraining.InceptionTransferLeaner(model_name) | ||
new_model, label_dict, history = this_IV3_transfer.transfer_model(image_data_path, | ||
print "Done loading model" |
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.
log instead of pring
Added Flask blueprint to handle sentiment prediction request. Added sentiment server to read text from TEXT_QUEUE. Use as below -
curl -X POST
http://127.0.0.1:3031/sentimentV1/predict
-H 'Cache-Control: no-cache'
-H 'Postman-Token: eeedb319-2218-44b9-86eb-63a3a1f62e14'
-H 'content-type: multipart/form-data; boundary=----WebKitFormBoundary7MA4YWxkTrZu0gW'
-F textv='the movie is good'
-F model_name=base