-
Notifications
You must be signed in to change notification settings - Fork 351
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
Fix BatchMemoryManager length #641
Conversation
@facebook-github-bot has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
@facebook-github-bot has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
@facebook-github-bot has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
@facebook-github-bot has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you add this missing line back? The current version fails the lint test (https://app.circleci.com/pipelines/github/pytorch/opacus/3256/workflows/26b31f87-1e6e-4562-b070-59dc983bdf4e/jobs/18514?utm_campaign=vcs-integration-link&utm_medium=referral&utm_source=github-checks-link&utm_content=summary). Then I will approve the pr. Thx!
@facebook-github-bot has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
Sorry my styling constantly changed everything around. Files changed now shows only two changed lines, hope it works now! |
This pull request has been merged in 7d65ddf. |
Types of changes
Motivation and Context / Related issue
Fixes #640 by ceiling the number of batches.
How Has This Been Tested (if it applies)
Checklist