simclr fixes #329

ananyahjha93 · 2020-11-02T13:55:53Z

What does this PR do?

1 PR to fix a bunch of SimCLR issues.

Fixes #318
Fixes #227
Fixes #320
Fixes #322
Fixes #327
Fixes #179
Fixes #309
Fixes #355
Fixes #357

Before submitting

Was this discussed/approved via a Github issue? (no need for typos and docs improvements)
Did you read the contributor guideline, Pull Request section?
Did you make sure your PR does only one thing, instead of bundling different changes together? Otherwise, we ask you to create a separate PR for every change.
Did you make sure to update the documentation with your changes?
Did you write any new necessary tests?
Did you verify new and existing tests pass locally with your changes?
If you made a notable change (that affects users), did you update the CHANGELOG?

PR review

Is this pull request ready for review? (if not, please submit in draft mode)

Anyone in the community is free to review the PR once the tests have passed.
If we didn't discuss your PR in Github issues there's a high chance it will not be merged.

Did you have fun?

Make sure you had fun coding 🙃

codecov · 2020-11-02T13:56:13Z

Codecov Report

Merging #329 (634a050) into master (b746be0) will decrease coverage by 0.81%.
The diff coverage is 70.81%.

@@            Coverage Diff             @@
##           master     #329      +/-   ##
==========================================
- Coverage   82.00%   81.18%   -0.82%     
==========================================
  Files         100      100              
  Lines        5639     5715      +76     
==========================================
+ Hits         4624     4640      +16     
- Misses       1015     1075      +60

Flag	Coverage Δ
cpu	`24.28% <16.88%> (-0.27%)`	⬇️
pytest	`24.28% <16.88%> (-0.27%)`	⬇️
unittests	`80.47% <70.81%> (-0.79%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
pl_bolts/callbacks/ssl_online.py	`100.00% <ø> (ø)`
pl_bolts/models/self_supervised/byol/models.py	`100.00% <ø> (ø)`
...olts/models/self_supervised/swav/swav_finetuner.py	`100.00% <ø> (ø)`
...l_bolts/models/self_supervised/swav/swav_module.py	`66.79% <ø> (ø)`
...lts/models/self_supervised/simclr/simclr_module.py	`71.63% <68.36%> (-11.18%)`	⬇️
..._bolts/models/self_supervised/simclr/transforms.py	`80.88% <69.23%> (-19.12%)`	⬇️
...l_bolts/models/self_supervised/byol/byol_module.py	`83.90% <100.00%> (-1.04%)`	⬇️
pl_bolts/models/self_supervised/resnets.py	`92.04% <100.00%> (-0.74%)`	⬇️
.../models/self_supervised/simclr/simclr_finetuner.py	`100.00% <100.00%> (ø)`
pl_bolts/losses/self_supervised_learning.py	`71.33% <0.00%> (-6.37%)`	⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b746be0...634a050. Read the comment docs.

Borda · 2020-11-06T13:44:22Z

@ananyahjha93 how is it going here, seems like very important fix...

pep8speaks · 2020-11-13T17:29:44Z

Hello @ananyahjha93! Thanks for updating this PR.

There are currently no PEP 8 issues detected in this Pull Request. Cheers! 🍻

Comment last updated at 2020-11-17 18:09:42 UTC

Borda

pls check the argparser issues

pl_bolts/models/self_supervised/resnets.py

pl_bolts/models/self_supervised/simclr/simclr_finetuner.py

pl_bolts/models/self_supervised/simclr/simclr_module.py

SeanNaren

I'll trust you on the model logic, everything seems fine. Followup conversation about hparams should happen @tchaton @Borda

Borda · 2020-11-17T19:16:04Z

pl_bolts/models/self_supervised/simclr/simclr_finetuner.py

-        dm.val_transforms = SimCLREvalDataTransform(32)
-        dm.test_transforms = SimCLREvalDataTransform(32)
-        args.num_samples = dm.num_samples
+        dm = CIFAR10DataModule(


can we simplify this one, wrap it as a func which does this hard overwrite?

Borda · 2020-11-17T19:17:13Z

pl_bolts/models/self_supervised/simclr/simclr_finetuner.py

+            num_workers=args.num_workers
+        )
+
+        dm.train_transforms = SimCLRFinetuneTransform(


well in fact this shall be in the dm as def set_finetune_transfroms(self) -> None

maybe @akihironitta could take it in another PR...

Borda · 2020-11-17T19:18:28Z

pl_bolts/models/self_supervised/simclr/simclr_module.py

-from pl_bolts.models.self_supervised.simclr.transforms import SimCLREvalDataTransform, SimCLRTrainDataTransform
-from pl_bolts.optimizers.lars_scheduling import LARSWrapper
-from pl_bolts.optimizers.lr_scheduler import LinearWarmupCosineAnnealingLR
+class SyncFunction(torch.autograd.Function):


can we add docs to what it does?

Borda · 2020-11-17T19:20:53Z

pl_bolts/models/self_supervised/simclr/simclr_module.py

+        if self.arch == 'resnet18':
+            backbone = resnet18
+        elif self.arch == 'resnet50':
+            backbone = resnet50


Suggested change

if self.arch == 'resnet18':

backbone = resnet18

elif self.arch == 'resnet50':

backbone = resnet50

backbones = {

'resnet18': resnet18,

'resnet50': resnet50,

}

backbone = backbones[self.arch]

Borda · 2020-11-17T19:23:40Z

pl_bolts/models/self_supervised/simclr/simclr_module.py

+
+    def nt_xent_loss(self, out_1, out_2, temperature, eps=1e-6):
+        """
+            assume out_1 and out_2 are normalized


Suggested change

assume out_1 and out_2 are normalized

Args:

assume out_1 and out_2 are normalized

Borda · 2020-11-17T19:24:31Z

pl_bolts/models/self_supervised/simclr/simclr_module.py

-        parser.add_argument('--meta_dir', default='.', type=str, help='path to meta.bin for imagenet')
+
+        # model params
+        parser.add_argument("--arch", default="resnet50", type=str, help="convnet architecture")


not for PL, this could be taken automatically from Model init arguments...

Borda · 2020-11-17T19:25:48Z

pl_bolts/models/self_supervised/simclr/simclr_module.py

-        dm.val_transforms = SimCLREvalDataTransform(32)
-        args.num_samples = dm.num_samples
+    if args.dataset == 'stl10':
+        dm = STL10DataModule(


can we wrap these hardcoded dataset decedent changes in a function?

Borda · 2020-11-17T19:26:55Z

pl_bolts/models/self_supervised/simclr/transforms.py

-            )
-
+    # Implements Gaussian blur as described in the SimCLR paper
+    def __init__(self, kernel_size, p=0.5, min=0.1, max=2.0):


Suggested change

def __init__(self, kernel_size, p=0.5, min=0.1, max=2.0):

def __init__(self, kernel_size: int, p: float = 0.5, min: float = 0.1, max: float = 2.0):

pls use longer var then p and do not overwrite native func like min/max

still to do...

Borda · 2020-11-17T23:17:53Z

just for the record @ananyahjha93 promised to fix all comments above in another PR 🐰

Borda added the fix fixing issues... label Nov 3, 2020

Borda requested review from williamFalcon and Borda November 3, 2020 18:24

Borda added model Priority High priority task labels Nov 6, 2020

ananyahjha93 force-pushed the fix/simclr branch from d21b54a to b6ee9da Compare November 11, 2020 22:04

ananyahjha93 changed the title ~~[wip] simclr fixes~~ simclr fixes Nov 16, 2020

ananyahjha93 mentioned this pull request Nov 16, 2020

How to use SSLOnlineEvaluator callback #309

Closed

ananyahjha93 changed the title ~~simclr fixes~~ [wip] simclr fixes Nov 16, 2020

Borda previously requested changes Nov 16, 2020

View reviewed changes

ananyahjha93 force-pushed the fix/simclr branch from e018bc4 to 2fb1f23 Compare November 16, 2020 18:25

ananyahjha93 added 16 commits November 16, 2020 13:45

initial

91e3034

updated transform

1fe59ba

changes

e0053be

updates simclr

0f178de

added to docs

dab4370

fix imports

71504a2

updated transforms

5c81197

encoder fix

a21ddb7

encoder fix

20fdce1

.

254b236

.

673a56e

resnet update

f3e5769

resnet

14877fa

.

9402ae4

fix transforms

af04225

fix

8e3d246

ananyahjha93 added 6 commits November 16, 2020 13:45

fix tests

3486b2a

fix tests

0ac1ef8

fix tests

77626d7

fix tests

b080ca0

updated swav stl10 path

b1cb38a

updated nt_xent_loss

b8595eb

ananyahjha93 force-pushed the fix/simclr branch from 2fb1f23 to b8595eb Compare November 16, 2020 18:46

ananyahjha93 added 4 commits November 16, 2020 14:51

grad version of all_reduce

dfe8e7d

grad version of all_reduce

3e2b8f3

pep8

f7bc230

syncfunc with all_redice

c1ced0e

ananyahjha93 force-pushed the fix/simclr branch from 47e8fdf to c1ced0e Compare November 17, 2020 03:02

ananyahjha93 changed the title ~~[wip] simclr fixes~~ simclr fixes Nov 17, 2020

teddykoker approved these changes Nov 17, 2020

View reviewed changes

ananyahjha93 added 4 commits November 17, 2020 11:28

fix pep8

786f570

sum at sync

07bdb1e

change grad sync to sum op

77de560

doc fixes

634a050

SeanNaren approved these changes Nov 17, 2020

View reviewed changes

SeanNaren requested a review from Borda November 17, 2020 19:14

Borda reviewed Nov 17, 2020

View reviewed changes

Borda self-requested a review November 17, 2020 19:28

ananyahjha93 merged commit 77ff983 into master Nov 17, 2020

ananyahjha93 deleted the fix/simclr branch November 17, 2020 19:36

Borda added this to Done in Self Supervised Learning Nov 20, 2020

Borda added this to the v0.3 milestone Jan 18, 2021

This was referenced Mar 13, 2021

[tune](deps): Bump pytorch-lightning-bolts from 0.2.5 to 0.3.0 in /python/requirements suquark/ray#8

Closed

[tune](deps): Bump pytorch-lightning-bolts from 0.2.5 to 0.3.0 in /python/requirements sven1977/ray#8

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

simclr fixes #329

simclr fixes #329

ananyahjha93 commented Nov 2, 2020 •

edited

Loading

codecov bot commented Nov 2, 2020 •

edited

Loading

Borda commented Nov 6, 2020

pep8speaks commented Nov 13, 2020 •

edited

Loading

Borda left a comment

SeanNaren left a comment

Borda Nov 17, 2020

Borda Nov 17, 2020

Borda Nov 17, 2020

Borda Nov 17, 2020

Borda Nov 17, 2020

Borda Nov 17, 2020

Borda Nov 17, 2020

Borda Nov 17, 2020

Borda Nov 17, 2020

Borda commented Nov 17, 2020

	assume out_1 and out_2 are normalized
	Args:
	assume out_1 and out_2 are normalized

	def __init__(self, kernel_size, p=0.5, min=0.1, max=2.0):
	def __init__(self, kernel_size: int, p: float = 0.5, min: float = 0.1, max: float = 2.0):

simclr fixes #329

simclr fixes #329

Conversation

ananyahjha93 commented Nov 2, 2020 • edited Loading

What does this PR do?

Before submitting

PR review

Did you have fun?

codecov bot commented Nov 2, 2020 • edited Loading

Codecov Report

Borda commented Nov 6, 2020

pep8speaks commented Nov 13, 2020 • edited Loading

Comment last updated at 2020-11-17 18:09:42 UTC

Borda left a comment

Choose a reason for hiding this comment

SeanNaren left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Borda commented Nov 17, 2020

ananyahjha93 commented Nov 2, 2020 •

edited

Loading

codecov bot commented Nov 2, 2020 •

edited

Loading

pep8speaks commented Nov 13, 2020 •

edited

Loading