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

Added VicReg - Variance-Invariance-Covariance Regularization for SSL #1030

Open
wants to merge 54 commits into
base: master
Choose a base branch
from

Conversation

dewball345
Copy link
Contributor

@dewball345 dewball345 commented Aug 15, 2022

This is a reupload(sorry about that). Saw that the original PR was a little hard to find so I created a new updated one.

@owenvallis approved the code but it needs @fchollet's approval to merge into main.

For reference here is the original PR: #847

@dewball345
Copy link
Contributor Author

@dewball345 , Could you please update on this PR. Thanks!

@sachinprasadhs I really want to thank you for letting me know with regards to updates - i completely overlooked this PR and the change that was requested by @fchollet.

@dewball345
Copy link
Contributor Author

@sachinprasadhs @fchollet I rebased the PR accordingly and it should be fine now.

@pcoet
Copy link
Collaborator

pcoet commented Sep 4, 2023

@dewball345 I see the following request from @fchollet:

Also, please only include the .py file in the PR until the PR is approved (since we would have to regenerate the other files at that time). There should only be one file in the "modified files" tab. Thank you!

Can you submit the .py file only, so that @fchollet can review?

@dewball345
Copy link
Contributor Author

dewball345 commented Sep 4, 2023

@pcoet I did, the barlow twins files were files that I brought back by rebasing my PR(there were some weird git related things that occurred and the barlow twins notebooks were deleted in this PR so I had to bring them back by a rebase). Those aren't new files, I just brought them back. Only vicreg.py is the file related to vicreg.

@pcoet
Copy link
Collaborator

pcoet commented Sep 4, 2023

Great, thanks @dewball345! I'll go ahead and assign this to @fchollet.

@pcoet pcoet assigned fchollet and unassigned pcoet Sep 4, 2023
@dewball345
Copy link
Contributor Author

Any update on this? @pcoet @fchollet

Copy link
Collaborator

@pcoet pcoet left a comment

Choose a reason for hiding this comment

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

@dewball345 Thanks for the ping on this. I made a couple of nitty suggestions on the Markdown content. Can you make those changes, and then we'll ask @fchollet to do a review?



"""
### From-Scratch implementation(for understanding purposes)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Missing space: "implementation(for" -> "implementation (for"

Modified from
Responsible for the Resnet 34 architecture.
Modified from
https://www.analyticsvidhya.com/blog/2021/08/how-to-code-your-resnet-from-scratch-in-tensorflow/#h2_2.
Copy link
Collaborator

Choose a reason for hiding this comment

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

It looks like there's a copy-paste error here(?). Is this the same URL repeated several times? Please check and clean up, as needed.

@@ -605,9 +603,15 @@ def plot_values(batch: tuple):

After this the two parts are summed together.

We will be using the [BarlowLoss](https://github.com/tensorflow/similarity/blob/master/tensorflow_similarity/lo
sses/barlow.py)
Copy link
Collaborator

Choose a reason for hiding this comment

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

The Markdown link can't be broken across lines. Please keep the entire link on a single line. See https://github.com/keras-team/keras-io/blob/master/contributor_guide.md#markdown-links.

primarily store information from the images that differentiate the cat from its canine
counterpart. For example, it could keep the shape of the ears of both images, or maybe
the tail length, and so on. When used in a downstream task,
like a classification model, these embeddings(which have the curated measurements
Copy link
Collaborator

Choose a reason for hiding this comment

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

Missing space: "embeddings(which" -> "embeddings (which".

[Barlow Twins Paper](https://arxiv.org/abs/2103.03230)

[Barlow Twins Example(Some of the architecture code is copied from
it)](https://keras.io/examples/vision/barlow_twins/)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Markdown links have to be on a single line.

@pcoet
Copy link
Collaborator

pcoet commented Jan 2, 2024

@dewball345 Also, I apologize for not requesting these changes earlier. Thanks for your patience!

@dewball345
Copy link
Contributor Author

@pcoet Hi, it has been a while. I forgot about this PR for a while but I went back and made the fixes. Please have a look

@sachinprasadhs
Copy link
Collaborator

@dewball345 , We have migrated from Keras 2 to Keras 3, could you please consider migrating this tutorial to Keras 3.
For more details visit https://keras.io/guides/migrating_to_keras_3/

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants