Nightly dev to Dev #3
Conversation
models/evaluator.py
Outdated
| from nltk.translate.bleu_score import sentence_bleu | ||
|
|
||
|
|
||
| def evaluation(translations, references): |
There was a problem hiding this comment.
why not just use, gold and prediction here instead of translations and references variables?
There was a problem hiding this comment.
translations refers to translated sentence and references refers to source sentences,can you more explain about gold and prediction?
There was a problem hiding this comment.
so based on your message, prediction is translations and gold is references (for gold naming you can use truth) too
models/evaluator.py
Outdated
| print('Individual 4-gram: %f' % sentence_bleu(refer_token, trans_token, weights=(0, 0, 0, 1))) | ||
|
|
||
|
|
||
| # reference = [['i love cats'], ['i love hats']] |
There was a problem hiding this comment.
you can put this sample input and outputs inside of the evaluator function!
There was a problem hiding this comment.
# reference = [['i love cats'], ['i love hats']] ..... which you are using, please put them inside of the evaluator function docstring.
models/evaluator.py
Outdated
| from nltk.translate.bleu_score import sentence_bleu | ||
|
|
||
|
|
||
| def evaluation(translations, references): |
There was a problem hiding this comment.
what is the name of this evaluation, you can use its name here, for example, bleu_evaluation not only simple evaluation naming
models/evaluator.py
Outdated
| Returns: | ||
| nothings for now, just print BLEU scores | ||
| """ | ||
| print('translations=[%s], references=%s' % (translations, references)) |
There was a problem hiding this comment.
please put a verbose variable to whatever to do a print or not! printing all of those lists could be tricky
for example
def evaluation(...,..., verbose=False):
if verbose:
print("..............")
...
...
models/evaluator.py
Outdated
| for sen in references: | ||
| refer_token.append(sen[0].split()) | ||
|
|
||
| print('Individual 1-gram: %f' % sentence_bleu(refer_token, trans_token, weights=(1, 0, 0, 0))) |
There was a problem hiding this comment.
Individual 1-gram what score??? is this BLEU so put its in print function!
There was a problem hiding this comment.
print('BLEU, Individual 1-gram: %f' % sentence_bleu(refer_token, trans_token, weights=(1, 0, 0, 0)))
models/model_utils.py
Outdated
| from preprocessing import create_dataset | ||
| from sklearn.model_selection import train_test_split | ||
|
|
||
| path_to_file = '/media/moslem/Private/Project/MTP/Machine-Translation/dataset/manythings/pes.txt' |
There was a problem hiding this comment.
is it a valid path to file in this repository? and is it required to pass here?
models/model_utils.py
Outdated
| convert(targ_lang_tokenizer, target_tensor_train[0]) | ||
|
|
||
|
|
||
| main_proccess() |
|
put documentation into all of your methods too! I saw a few of them don't have it! |
models/evaluator.py
Outdated
| candidate = 'i love cat' | ||
| bleu_evaluation(candidate, truth) | ||
| """ | ||
| print('translations=[%s], references=%s' % (prediction, truth)) |
There was a problem hiding this comment.
I think you should add verbose to this print, not to the other ones, since the amount of prediction and truth variables could be more than just one sentence it can be annoying to print all of them
models/evaluator.py
Outdated
| for sen in prediction: | ||
| truth_token.append(sen[0].split()) | ||
|
|
||
| if verbose: |
There was a problem hiding this comment.
no need verbose for this prints
|
dear @HamedBabaei i read your comments and push the new version, Thanks |
dear @HamedBabaei please check this merge request,sorry for delay.