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

Vibcreg #182

Merged
merged 18 commits into from
Nov 22, 2021
Merged

Vibcreg #182

merged 18 commits into from
Nov 22, 2021

Conversation

Froskekongen
Copy link
Contributor

@Froskekongen Froskekongen commented Nov 2, 2021

Implementation of VIbCReg (https://arxiv.org/abs/2109.00783), which was originally developed for times series modeling.

The performance without tuning parameters and using the same parameters as VICReg gets a performance of 91.18 acc@1 online, and 99.74 acc@5 online.

@vturrisi
Copy link
Owner

vturrisi commented Nov 2, 2021

Hey @Froskekongen thanks a lot for contributing. I'll take a look at the paper first to get the general idea.
It might take longer than normal because of CVPR, but I'll try to give some feedback as soon as I can.

@DonkeyShot21
Copy link
Collaborator

Hi @Froskekongen! We appreciate your initiative.

Did you happen to run CIFAR100 as well?

@Froskekongen
Copy link
Contributor Author

Froskekongen commented Nov 2, 2021

@DonkeyShot21: I will run CIFAR-100 overnight.

EDIT: I will run it overnight if I manage to download the dataset. There appears to be a 500 error from the site hosting the dataset right now.

Copy link
Owner

@vturrisi vturrisi left a comment

Choose a reason for hiding this comment

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

I found some small things that would be good to be fixed before merging.
Also, can you add the loss to solo/losses/__init__.py?

TODO:

  • implement tests
  • add method to the docs
  • add tests
  • add results/checkpoints to README
  • black files (the bot seems to have failed for some reason)

solo/losses/vibcreg.py Show resolved Hide resolved
solo/methods/vibcreg.py Show resolved Hide resolved
solo/methods/vibcreg.py Outdated Show resolved Hide resolved
solo/methods/vibcreg.py Outdated Show resolved Hide resolved
solo/utils/whitening.py Outdated Show resolved Hide resolved
solo/utils/whitening.py Outdated Show resolved Hide resolved
solo/utils/whitening.py Outdated Show resolved Hide resolved
solo/utils/whitening.py Outdated Show resolved Hide resolved
solo/methods/vibcreg.py Outdated Show resolved Hide resolved
) # whiten matrix: the matrix inverse of Sigma, i.e., Sigma^{-1/2}

running_mean.copy_(momentum * mean + (1.0 - momentum) * running_mean)
running_wmat.copy_(momentum * wm + (1.0 - momentum) * running_wmat)
Copy link
Owner

Choose a reason for hiding this comment

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

I'm getting this error. It's probably related to the clone() that I said we could remove. Sorry about that.

E RuntimeError: unsupported operation: more than one element of the written-to tensor refers to a single memory location. Please clone() the tensor before performing the operation.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I reintroduced the .clone().

@vturrisi
Copy link
Owner

vturrisi commented Nov 2, 2021

@Froskekongen I cleaned a bit, added some placeholders for results in the readme and fixed part of the tests. We are still getting an error which I assume is due to the clone() that I asked you to remove. We are also missing documentations (me or @DonkeyShot21 can add later) and the results/checkpoints for CIFAR10/100 and Imagenet-100.

@codecov
Copy link

codecov bot commented Nov 3, 2021

Codecov Report

Merging #182 (885bf8c) into main (a829643) will decrease coverage by 0.10%.
The diff coverage is 94.11%.

Flag Coverage Δ *Carryforward flag
cpu 84.56% <94.64%> (+0.59%) ⬆️
dali 42.24% <80.00%> (ø) Carriedforward from dae451c

*This pull request uses carry forward flags. Click here to find out more.

Impacted Files Coverage Δ
solo/utils/whitening.py 88.14% <90.47%> (-11.86%) ⬇️
solo/losses/__init__.py 100.00% <100.00%> (ø)
solo/losses/vibcreg.py 100.00% <100.00%> (ø)
solo/methods/__init__.py 93.10% <100.00%> (-6.90%) ⬇️
solo/methods/vibcreg.py 100.00% <100.00%> (ø)

@vturrisi
Copy link
Owner

vturrisi commented Nov 3, 2021

I'll take care of the documents. @Froskekongen are you running/planning to run the experiments?

@Froskekongen
Copy link
Contributor Author

@vturrisi: I ran the experiments for cifar10 and cifar100
cifar10: val_acc1 - 91.18, val_acc5 - 99.74
cifar100: val_acc1 - 67.37, val_acc5 - 90.07

@vturrisi
Copy link
Owner

vturrisi commented Nov 3, 2021

Ok! I'll create some folders so that you can upload the checkpoints, k? About imagenet100, are you running or planning to run?

@Froskekongen
Copy link
Contributor Author

Yes, I can upload the trained models to some folders. I didn't plan to run imagenet100 - at least not right now.

@vturrisi
Copy link
Owner

vturrisi commented Nov 8, 2021

@Froskekongen just to confirm we are missing the checkpoints and Imagenet-100 results.
You can add the cifar10 checkpoint here: https://drive.google.com/drive/folders/19u3p1maX3xqwoCHNrqSDb98J5fRvd_6v?usp=sharing and cifar100 here: https://drive.google.com/drive/folders/19u3p1maX3xqwoCHNrqSDb98J5fRvd_6v?usp=sharing, both the args.json file and the checkpoint itself. When I'm able to run imagenet-100, I'll upload everything and we merge the PR.

EDIT: just realised that I put the same link twice. I already moved the files to the correct folders.

@Froskekongen
Copy link
Contributor Author

I have added the args and checkpoints to the drive folder.

@vturrisi
Copy link
Owner

Hey @Froskekongen. I'll run on imagenet-100. Can I use the same parameters as in VICReg?

@Froskekongen
Copy link
Contributor Author

@vturrisi: The only thing that needs to be changed are the loss weights, which should be:

    --sim_loss_weight 25.0 \
    --var_loss_weight 25.0 \
    --cov_loss_weight 200.0 \

@vturrisi
Copy link
Owner

@Froskekongen just started with imagenet-100. When this is done and I run offline linear eval, I'll merge the PR (that should be tomorrow or Monday).

@vturrisi
Copy link
Owner

Merging it now. Thanks again @Froskekongen for contributing.

@vturrisi vturrisi merged commit 9e725ad into vturrisi:main Nov 22, 2021
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.

None yet

3 participants