Added TQDM Progressbar for evaluate()

[ashutosh1919]

fixes #1620 .
Although, I couldn't figure out the use of batch parameter in on_test_batch_begin method.
@zaccharieramzi and @gabrieldemarmiesse , please review and suggest changes.

[shun-lin]

May you also update the test cases in https://github.com/tensorflow/addons/blob/master/tensorflow_addons/callbacks/tests/tqdm_progress_bar_test.py ? Thank you so much!

[ashutosh1919]

@shun-lin , please suggest changes. I have got the problem. I think we need to add 4 methods : on_test_begin() (To initialize tqdm params) , on_test_batch_begin() (Probably to show batch num), on_test_batch_end() (To update progressbar with the info) and on_test_end() (to close progress bar)

[shun-lin]

@shun-lin , please suggest changes. I have got the problem. I think we need to add 4 methods : on_test_begin() (To initialize tqdm params) , on_test_batch_begin() (Probably to show batch num), on_test_batch_end() (To update progressbar with the info) and on_test_end() (to close progress bar)

Thanks! I agree with you that those are the methods that we need to add (and maybe for better readability we probably want to refactor the on_test|train_* code so it wouldn't have duplicate, but that can be done later). So because Model.evaluate() is equivalent to running 1 epoch over the testing dataset (see screencast below), we can utilize how we show the progress within each epoch, see self.epoch_progress_tqdm and how we update it in on_batch_end method (maybe the evaluate will call on_batch_end if on_test_batch_end is absent? (double check this please thanks!, if this is not the case then just extract out the logic in on_batch_end and call that logic with on_test_batch_end too). Therefore on_test_begin() should look like on_epoch_begin and on_test_end should look like on_epoch_end (since there is no need for overall progress bar for evaluate).

Let me know if this is confusing to you, happy to explain it further! Thanks for the contribution!

[ashutosh1919]

@shun-lin ,
Small Help Required :

The progress :
I was successfully able to produce the evaluate progress bar as shown below but the implementation was little bit modification of what you described in the previous comment. Below are the lessons learnt during implementation.

• We have to use exactly three methods to do what is required : on_test_begin() (to initialize the prograss bar and params for progress bar), on_test_batch_end() (to update progress bar after each batch is evaluated), on_test_end() (to update final metrices and close tqdm progress bar).
• We can not just copy the code from on_epoch_begin() and on_batch_end() because, the self.x types of variables are shared across whole class. It won't get affected during call of evaluate() method, but it will produce wrong results when fit() is called and validation_data is provided. So, separate copies of every param is required for this new progress bar.

Now, I have added code such that it will produce progress bar both in case of evaluate() method and also in case of fit() with validation_data defined.

The Problems :
But there are still couple of problems that I have noticed.

• You can see in above image that there is separate list printed containing loss and acc below progress bar. But they must be defined in tqdm.desc (At the end of progress bar). I am searching, but still not able to locate the code which prints that list. Also, not able to find the reason why the desc is printing loss at the time when progress bar is filling but desc is not printing loss when progress bar is whole filled.
• Right now, the progress bar is using the steps taken from self.params["steps"] which is same as in training. Again, this won't affect evaluate(), But in case of fit() with validation_data, self.params["steps"] will be shared which should not be. We need to create something to calculate total steps of validation_data.

Sorry for long comment, the demo of current progress bar is here. Please review it, and suggest changes to resolve problems. Thanks for help!

[shun-lin]

@ashutosh1919 great progress and good observations! Couple of remarks.

1. The separate list of loss and accuracy is expected, this is the return of model.evaluate() and because notebook display the return of last line in a cell there is nothing wrong with it. Here is the reference: https://www.tensorflow.org/api_docs/python/tf/keras/Model#evaluate

if you want the extra list to not print, just simply assign it to a variable, metric_lst = model.evaluate(...).

2. I noticed that you have added dynamic_col=True, which should have solved the problem that the bar (the green part) is too long, but in your screenshot it's still too long, may you look into the version of your tqdm and check if updated version would help?

3. In terms of printing progress bar for train with validation_data, I would suggest to follow how the original progress bar is doing. The original progress bar did not display a separate progress bar when computing metrics over data in validation_data, this behavior is different with evaluate(). Therefore we need to distinguish the case when the method is called from evaluate() vs train with validation_data. I think this helps with your 2nd problem. Let me know if this is not clear, thanks!

Sincerely,
Shun Lin

[ashutosh1919]

@shun-lin , yes you are right. I was thinking the same that we should not display another progress bar in case of train with validation_data. But is there any parameter in logs which is passed from evaluate() or fit() which will help me distinguish these two cases?

But I have very strange temporary idea which I don't think is good :

if(nb_epoch==1):
    # Came from evaluate()
else:
    # Came from fit()

[shun-lin]

@ashutosh1919 I would suggest to do the refactoring to remove duplicate code in this PR if you think it's not too much work to do so (I think it's easier to do it in this PR than in the future PR).

[ashutosh1919]

@ashutosh1919 I would suggest to do the refactoring to remove duplicate code in this PR if you think it's not too much work to do so (I think it's easier to do it in this PR than in the future PR).

Sure will try to do that today itself then.

[ashutosh1919]

@shun-lin , I have refactored the existing code to remove duplicity. I have kept the function names temporarily. They can be changed if you say so. Please review it and suggest changes. I have tested the functionality in Google colab notebook. It works fine.

[shun-lin]

Thanks so much for your time and contribution!! I have left quite a few comments (mostly on method/variable names), after resolving those comments its pretty much LGTM from me, thanks!

[shun-lin]

2 little comments, thanks so much!

[shun-lin]

I have one tiny tiny NIT, but other than everything looks good! Thanks so much for the contributions!

[shun-lin]

Everything looks good, thanks for your contribution!! :)

[ashutosh1919]

sorry, didn't see your comment on readability. Please sorry for this. Can you approve it again?

[shun-lin]

Everything looks good! :)

[shun-lin]

sorry, didn't see your comment on readability. Please sorry for this. Can you approve it again?

approved! Will merge after the tests finished running, thanks so much @ashutosh1919

[ashutosh1919]

@shun-lin , all tests are passing. Merge it anytime if you are fine with it.

[gabrieldemarmiesse]

Looks nice, thanks!