Open alessiamarcolini opened 4 years ago
Hey @sthalles I saw you updated simcrl.py
in the last commit, but the behavior of the code is the same as before: counter
starts from 0.
Here the problem is not only the ZeroDivisionError
(in case you don't have enough validation samples to fill a validation batch - so the for loop never runs), but also that at the end you get a wrong valid_loss
.
Let me show you an example with numbers:
counter
will range from 0 to 4 (included).valid_loss
, so at the end of the 5 iterations valid_loss
will be equal to 10.If at the end you divide valid_loss
by counter
, you'll get 10/4, resulting 2.5, and not 10/5 resulting 2, as expected.
That's why in this PR I changed the divisor to counter + 1
.
Now, I see that simclr.py
conflicts with my changes: do you want me to resolve the conflict and push again?
Hi Alessia,
Thanks for the message. About the zero division, that is true and easy to solve, I will fix it. Regarding the validation loss calculation, I did not push the code because actually dividing by (counter + 1) would be wrong. That follows a simple code with the example you described above. Regards
counter = 0 # counter needs to start at zero valid_loss = 0.0
for i in range(5): print("Batch {}".format(i)) print("----") valid_loss += 2 counter += 1
print("valid_loss:", valid_loss) print("counter:", counter) print("Final valid loss:", valid_loss / counter)
Batch 0 ---- Batch 1 ---- Batch 2 ---- Batch 3 ---- Batch 4 ---- valid_loss: 10.0 counter: 5 Final valid loss: 2.0
-- Thalles Santos Silva
Computer Vision and Deep Learning engineer.
+55 (73) 98222-8989
On Sat, Apr 18, 2020 at 12:21 PM Alessia Marcolini notifications@github.com wrote:
Hey @sthalles https://github.com/sthalles I saw you updated simcrl.py in the last commit https://github.com/sthalles/SimCLR/commit/c057471dd0d76f329e9bee9fce626a4f92990dae, but the behavior of the code is the same as before: counter starts from 0.
Here the problem is not only the ZeroDivisionError (in case you don't have enough validation samples to fill a validation batch - so the for loop never runs), but also that at the end you get a wrong valid_loss.
Let me show you an example with numbers:
- Say that you have 100 samples in your validation set and a batch size of 20, then you'll perform 5 iterations on the validation set.
- counter will range from 0 to 4 (included).
- Suppose that at each iteration you get a loss of 2 (silly example).
- You're accumulating the losses in valid_loss, so at the end of the 5 iterations valid_loss will be equal to 10.
If at the end you divide valid_loss by counter, you'll get 10/4, resulting 2.5, and not 10/5 resulting 2, as expected.
That's why in this PR I changed the divisor to counter + 1.
Now, I see that simclr.py conflicts with my changes: do you want me to resolve the conflict and push again?
— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/sthalles/SimCLR/pull/5#issuecomment-615888806, or unsubscribe https://github.com/notifications/unsubscribe-auth/AA2V5PFEEUVVKDBOYY4GBTTRNHAQDANCNFSM4MJNIBLQ .
Hi Talles,
thank you for your quick response.
Actually I was referring to counter
as the index coming from enumerate()
function, as in your previous revision, so now it actually works (although using two counter variables instead of one).
Regards
Fixes issue #4