fixed issues 61/62 #199
fixed issues 61/62 #199
Conversation
rfratila
added
bug fix
examples/cross_val_fashion.py
Outdated
| @@ -0,0 +1,77 @@ | |||
| """Simple Convolution and fully connected blocks example.""" | |||
There was a problem hiding this comment.
since this is an example script - can you add some more inline comments about what the various sections do? would be easier to follow
There was a problem hiding this comment.
that's a great start! but for each major block of code, we need comments so that you can be able to understand the entire flow of the example from just the inline docs. thanks!
| for epoch in iterator: | ||
| for epoch in range(epochs): | ||
|
|
||
| logger.info('\n -------- Epoch: {} --------\n'.format(epoch)) |
There was a problem hiding this comment.
why have this additional epoch log if the tqdm writer also writes the current epoch?
There was a problem hiding this comment.
Because I don't use tqdm on the epoch level anymore
There was a problem hiding this comment.
The problem was the overlapping progress bars, that's why i removed the epoch level one
There was a problem hiding this comment.
please see my other comment about this. I don't think the solution for this is to remove the progress bar altogether. unless I'm missing something, we just need to clean up the nested progress bar implementation.
| @@ -651,9 +652,11 @@ def fit(self, train_loader, val_loader, epochs, | |||
| else: | |||
| save_path = save_path + '/' + self.name + '_' | |||
| save_path = get_save_path(save_path, vis_type='train') | |||
| iterator = trange(epochs, desc='Epoch: ') | |||
| # iterator = trange(epochs, desc='Epoch: ') | |||
There was a problem hiding this comment.
doesn't this remove the progress bar at the epoch level?
There was a problem hiding this comment.
The problem was the overlapping progress bars, that's why i removed the outer one
There was a problem hiding this comment.
but you still need to print the epoch progress bar. Nested progress bars are what we are going for so they just need to be cleaned up in implementation.
closes #61 and closes #62