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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Memory bank distributed training support #1293

Conversation

guarin
Copy link
Contributor

@guarin guarin commented Jun 22, 2023

Changes

  • Add feature dimension when setting memory bank size. This is required for distributed training because the buffer size cannot be changed dynamically.
  • Cleanup memory bank implementation.
  • Add feature_dim_first argument to memory bank. This is required for backwards compatibility. In the future we should be able to set feature_dim_first=False and remove the requirement for transposing features coming from the feature bank.
  • Update classes/functions using memory bank from size=num_features to size=(num_features, dim).

How was it tested?

  • Added unit tests

@codecov
Copy link

codecov bot commented Jun 22, 2023

Codecov Report

Attention: 7 lines in your changes are missing coverage. Please review.

Comparison is base (18f13b2) 85.22% compared to head (d1c0063) 85.27%.

Files Patch % Lines
lightly/models/utils.py 0.00% 7 Missing ⚠️
Additional details and impacted files
@@                                Coverage Diff                                @@
##           guarin-lig-3062-add-mocov2-imagenet-benchmark    #1293      +/-   ##
=================================================================================
+ Coverage                                          85.22%   85.27%   +0.04%     
=================================================================================
  Files                                                130      130              
  Lines                                               5522     5533      +11     
=================================================================================
+ Hits                                                4706     4718      +12     
+ Misses                                               816      815       -1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@guarin guarin marked this pull request as ready for review November 24, 2023 15:32
Copy link
Contributor

@philippmwirth philippmwirth left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved, let's see how the MoCo benchmarks works now.

@guarin
Copy link
Contributor Author

guarin commented Dec 4, 2023

Merging this as the model is training well, see #1291

@guarin guarin merged commit 641ecf2 into guarin-lig-3062-add-mocov2-imagenet-benchmark Dec 4, 2023
9 of 10 checks passed
@guarin guarin deleted the guarin-memory-bank-ddp-support branch December 4, 2023 10:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants