Skip to content
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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

BatchGradientVerification should probably turn off batch norm #568

Closed
indigoviolet opened this issue Feb 23, 2021 · 3 comments 路 Fixed by #569
Closed

BatchGradientVerification should probably turn off batch norm #568

indigoviolet opened this issue Feb 23, 2021 · 3 comments 路 Fixed by #569
Assignees
Labels
bug Something isn't working help wanted Extra attention is needed

Comments

@indigoviolet
Copy link
Contributor

馃悰 Bug

In networks using Batch norm, there is "mixing" across the batch dimension but this probably doesn't indicate an error. Might be worth augmenting BatchGradientVerification and the associated callback to

(1) either always turn off batch norm, OR
(2) if we're about to return False, then check if turning it off will return True

Otherwise it can be super confusing to add this check and have it fail, forgetting about batch norm

To Reproduce

https://colab.research.google.com/drive/1dlY2vCBqMasrihIv_Y4DU-3BeKLd0Gox#scrollTo=fDRd_pRn_jL1

Expected behavior

See above.

cc @awaelchli

@indigoviolet indigoviolet added fix fixing issues... help wanted Extra attention is needed labels Feb 23, 2021
@github-actions
Copy link

Hi! thanks for your contribution!, great first issue!

@awaelchli
Copy link
Contributor

Hi @indigoviolet
Thanks for trying out my callback hihi <3
That's a very good observation! I was aware of it and also bumped into this myself a few times haha, just never had the time to address it properly. I should have made it clearer that it is a heuristic :) But of course an extension would be nice to support this. Your two proposed solutions are both reasonable imo. I would go with the first one since it's the simplest.

Do you have an interest in sending a PR?

@indigoviolet
Copy link
Contributor Author

cool, PR attached @awaelchli

@Borda Borda closed this as completed in #569 Mar 9, 2021
@Borda Borda added bug Something isn't working and removed fix fixing issues... labels Jun 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working help wanted Extra attention is needed
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants