-
Notifications
You must be signed in to change notification settings - Fork 4k
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
dump output when evaulating #2168
Conversation
No Taskcluster jobs started for this pull requestThe `allowPullRequests` configuration for this repository (in `.taskcluster.yml` on the
default branch) does not allow starting tasks for this pull request. |
8a6c65c
to
944f39c
Compare
Fixed the shared Queue issue |
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.
Looks good, but I have a few questions/comments.
evaluate_tflite.py
Outdated
work_todo.join() | ||
print('\nTotally %d work done' % work_done.qsize()) |
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.
Are the count
variable and these prints also debugging leftover?
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.
One of the redundant 'Total' can be removed if you think it's too much.
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 would keep just the latter I guess.
evaluate_tflite.py
Outdated
|
||
wer, cer, samples = calculate_report(ground_truths, predictions, losses) | ||
mean_loss = np.mean(losses) | ||
|
||
print('Test - WER: %f, CER: %f, loss: %f' % | ||
(wer, cer, mean_loss)) | ||
|
||
with open(args.csv + '.txt', 'w') as ftxt: | ||
with open(args.csv + '.out', 'w') as fout: |
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.
nit: you can comma separate context managers:
with open(args.csv + '.txt', 'w') as ftxt, open(args.csv + '.out', 'w') as fout:
# ...
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.
Thx. Will do
evaluate_tflite.py
Outdated
|
||
wer, cer, samples = calculate_report(ground_truths, predictions, losses) | ||
mean_loss = np.mean(losses) | ||
|
||
print('Test - WER: %f, CER: %f, loss: %f' % | ||
(wer, cer, mean_loss)) | ||
|
||
with open(args.csv + '.txt', 'w') as ftxt: |
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 it'd be better to condition this on some flag, as this behavior can be surprising, and also fail in surprising ways (e.g. dirname(args.csv)
may be read only).
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.
Sure
work_todo = JoinableQueue() # this is where we are going to store input data | ||
work_done = Queue() # this where we are gonna push them out | ||
work_done = manager.Queue() # this where we are gonna push them out |
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.
Nice catch, thanks for the fix.
944f39c
to
d63ecdd
Compare
@eggonlea can you fix the linter errors? Then we can merge this. |
Also dump output to a file Fixed some trivial pylint issues at the same time Signed-off-by: Li Li <eggonlea@msn.com>
d63ecdd
to
863c554
Compare
Fixed some legacy pylint issues as well. |
Thanks! |
This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs. |
Save the output for future use (e.g. calculate WER directly without running STT again).
And print the current progress.
Signed-off-by: Li Li eggonlea@msn.com