-
Notifications
You must be signed in to change notification settings - Fork 355
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
require python 3.7 + minor refactoring #277
Conversation
From offline discussion with @karthikprasad :
With that, we're choosing the oldest python 3.7 version available on circleCI: 3.7.4 |
Summary: This is an update to pytorch#277 to keep up with the changes of fb97c38. Also fix a link in Migration Guide along the way. Differential Revision: D32715608 fbshipit-source-id: c238a104ddbe74e2399888e680f378f764bd5ec8
Summary: Pull Request resolved: pytorch#293 This is an update to pytorch#277 to keep up with the changes of fb97c38. Also fix a link in Migration Guide along the way. Differential Revision: D32715608 fbshipit-source-id: ff3b9ed72e2254d9451f1623814cb11e4b331839
Summary: Pull Request resolved: pytorch#293 This is an update to pytorch#277 to keep up with the changes of fb97c38. Also fix a link in Migration Guide along the way. Differential Revision: D32715608 fbshipit-source-id: 6efc474eb9bbb424ad65a475d8c1484a384f0fd2
Following some internal test failures on #273, we've decided to up min required python version (we use annotations from future, which are only introduced in 3.7). Since 3.7 is already a default in colab, and is pretty popular overall, I don't see many potential issues with it.
One more change (which probably should have been decoupled into a separate pull request) is accountant hook refactoring. While writing guide on advanced usage of opacus, I realised that accountant hook is always the same for a given accountant. It can be different for different accountants, but same mechanism always requires the same hook.
This change makes it easier to attach accountant outside of PrivacyEngine