Added VicReg - Variance-Invariance-Covariance Regularization for SSL #847
Conversation
added link
sorry
Hi @fchollet, I did not include the actual file yet. Just have to do some grammar tweaks before I upload the notebook |
|
The changes to the Barlow files look good to me as well. I'll check back again once the VicReg files are uploaded. |
|
@owenvallis I've added the VicReg example. BTW - have used a custom fork due to some changes in augmenters. I've made a PR at the tf similarity repo so if that gets merged to either the dev or master branch I'll change the code to use it instead. |
|
@owenvallis @ebursztein could you check the VicReg code to see if everything looks good? |
| "outputs": [], | ||
| "source": [ | ||
| "!pip install tensorflow-addons" | ||
| "!!pip install tensorflow-addons\n", |
There was a problem hiding this comment.
Does this need the double !! for the shell command?
|
Small note on the pip install commands, but looks good to me otherwise. Thanks again for adding the update. |
|
Okay, I've made some changes - removed Barlow twins commits because those are unrelated to VicReg. Also, the TensorFlow similarity master branch is now used instead of the custom fork. Let me know if everything looks okay before I create an ipynb notebook |
Hey owen, have also added my VicReg files - LMK if they are okay and we can merge. |
owenvallis
left a comment
owenvallis
left a comment
There was a problem hiding this comment.
Looks good. I left a note that you might want to clean the output of the md and ipynb files as it looks like some code blocks printed a large number of lines.
examples/vision/md/vicreg.md
Outdated
| ``` | ||
| <div class="k-default-codeblock"> | ||
| ``` | ||
| ['Looking in indexes: https://pypi.org/simple, https://us-python.pkg.dev/colab-wheels/public/simple/', |
There was a problem hiding this comment.
I think we likely want to remove these repeated output lines.
There was a problem hiding this comment.
thanks, this looks better, although I would still probably remove most of the loss outputs. Maybe just keep just the first 2 and the last 2. Up to you though. You can always update the output if you change your mind.
@fchollet, what's the recommendation regarding printing loss outputs in the md and ipynb files?
There was a problem hiding this comment.
@owenvallis From what I found in the Keras io examples(including what I did in my previous examples), the loss outputs are kept as they are? Probably would be good to ask @fchollet but I'm open to including the first two and last two for readability purposes
There was a problem hiding this comment.
For now, just left the first two and last two, and made an indication that rest were skipped. If that's not the recommended way to do this will just revert to the previous commit
There was a problem hiding this comment.
@fchollet @owenvallis Any update on this?
There was a problem hiding this comment.
Added approval for the changes, but looks like you also need approval from someone with write access to the repo.
|
@fchollet Could you approve/give feedback on this PR - LMK if there are any issues with it. |
|
Closing and will bump this up |
3 participants
This is a draft PR - updated .py files will be added