[WIP] added cycleGAN notebook! #175

charchit7 · 2024-01-10T05:57:29Z

Hey everyone!

This PR adds CycleGAN Notebook!
Currently training the model is left.

Part of #28
Also, discussed in #158

Please take a look!
@johko @MKhalusova @merveenoyan

review-notebook-app · 2024-01-10T05:57:33Z

Check out this pull request on

See visual diffs & provide feedback on Jupyter Notebooks.

Powered by ReviewNB

merveenoyan · 2024-01-10T18:38:58Z

notebooks/Unit 5 - Generative Models/GANs & VAEs/cycleGAN.ipynb

@@ -0,0 +1,956 @@
+{


Can you remove step prefixes here so it's easier to read :)

Reply via ReviewNB

Sure, doing it now. Sorry for delay.

I think this is unaddressed

merveenoyan · 2024-01-10T18:38:58Z

notebooks/Unit 5 - Generative Models/GANs & VAEs/cycleGAN.ipynb

@@ -0,0 +1,956 @@
+{


Would you like to upload this dataset from Hub and load it from there? so when the link is broken this notebook doesn't break as well.

Reply via ReviewNB

Would be nice if you could use load_datasethttps://huggingface.co/docs/datasets/en/loading

+1 to using load_dataset

merveenoyan · 2024-01-10T18:38:59Z

notebooks/Unit 5 - Generative Models/GANs & VAEs/cycleGAN.ipynb

@@ -0,0 +1,956 @@
+{


Not sure you'd need this once this is loaded as HF dataset :)

Reply via ReviewNB

I wasn't able to figure out how to make this work with loading as Huggingface dataset, so this section is still needed. The best I could do was to download the dataset from Huggingface as a .zip

I think if you don't put the dataset to zip format (any other format works), it will automatically, converting it to parquet and you'd be able to load it

I've tried this with load_dataset as suggested but ran into issues - it fails in the train function.

Can you help take a look at my implementation / suggest changes I can try? https://colab.research.google.com/drive/1f328yVbuZmBjQRQhfOnC2QtrDFtCrXe_?usp=sharing

merveenoyan · 2024-01-10T18:38:59Z

notebooks/Unit 5 - Generative Models/GANs & VAEs/cycleGAN.ipynb

@@ -0,0 +1,956 @@
+{


great visual!!!

Reply via ReviewNB

Thank you, Merve!

Can you add some context for the image? Explain what it illustrates? Also, it comes right after you define the function to plot images, which doesn't make sense to me. It would be better to place in the introduction to the tutorial.

Moved the image and added context to it!

merveenoyan · 2024-01-10T18:38:59Z

notebooks/Unit 5 - Generative Models/GANs & VAEs/cycleGAN.ipynb

@@ -0,0 +1,956 @@
+{


Not super sure but I felt like you could decouple many of the layers in forward into another function so in forward you just call that and return last activation (completely up to you)

Reply via ReviewNB

merveenoyan · 2024-01-10T18:38:59Z

notebooks/Unit 5 - Generative Models/GANs & VAEs/cycleGAN.ipynb

@@ -0,0 +1,956 @@
+{


Losses*
no need to capitalize W

Reply via ReviewNB

merveenoyan · 2024-01-10T18:38:59Z

notebooks/Unit 5 - Generative Models/GANs & VAEs/cycleGAN.ipynb

@@ -0,0 +1,956 @@
+{


I think instead of having a long docstring you could use the markdown

Reply via ReviewNB

charchit7 · 2024-01-10T23:27:19Z

@hwaseem04 I left mostly formatting nits, otherwise it looks good!

Thanks @merveenoyan :)
I'll make the required changes.

I think you accidently tagged hwaseem04 😅

merveenoyan · 2024-01-11T11:22:46Z

ah sorry, removed.

charchit7 · 2024-01-11T11:24:52Z

@merveenoyan no problem at all. 🤗

sitamgithub-MSIT · 2024-01-18T21:57:55Z

@merveenoyan It's been a week for this PR. Also, suggested changes are committed as well. Should I assign myself as the reviewer and do a community review for this?

merveenoyan · 2024-01-19T11:16:06Z

@sitamgithub-MSIT hello, I don't see the changes incorporated unfortunately :/

charchit7 · 2024-01-19T11:29:38Z

@sitamgithub-MSIT I have yet to make changes.
@merveenoyan I had an accident last week and my right hand was cut a little. Will complete this by this weekend. Hope it's fine.

johko · 2024-01-19T20:18:46Z

no worries @charchit7, health definitely goes first

MKhalusova · 2024-01-23T15:23:35Z

notebooks/Unit 5 - Generative Models/GANs & VAEs/cycleGAN.ipynb

@@ -0,0 +1,956 @@
+{


Is this cell even needed?

Reply via ReviewNB

Thanks for calling out that there were 2 versions of the Generator class. Deleted the unused one!

MKhalusova · 2024-01-23T15:23:35Z

notebooks/Unit 5 - Generative Models/GANs & VAEs/cycleGAN.ipynb

@@ -0,0 +1,956 @@
+{


Line #2. '''
Looks like there was supposed to be a doctoring here.

Reply via ReviewNB

MKhalusova · 2024-01-23T15:23:35Z

notebooks/Unit 5 - Generative Models/GANs & VAEs/cycleGAN.ipynb

@@ -0,0 +1,956 @@
+{


Line #60. train()
After the training is finished, it would be nice to upload the model to Hub, and then show a quick inference example.

Reply via ReviewNB

I added a section below that uploads to Hub, but I wasn't able to figure out how to use the model for inference!

- Using Huggingface hosted dataset and push model to Hub - Use markdown instead of docstring

klyap · 2024-01-31T04:02:56Z

Hi! I'm helping @charchit7 out with wrapping up this PR and addressing review comments!

charchit7 · 2024-01-31T12:33:16Z

Thank you so much @klyap :)
Merve Et al. Please check :)

review-notebook-app · 2024-02-01T07:43:33Z

View / edit / reply to this conversation on ReviewNB

merveenoyan commented on 2024-02-01T07:43:33Z
----------------------------------------------------------------

just a small nit, can you also take the param names into single backtick to format them as code?

review-notebook-app · 2024-02-01T07:43:34Z

View / edit / reply to this conversation on ReviewNB

merveenoyan commented on 2024-02-01T07:43:34Z
----------------------------------------------------------------

If you put a gap between the # and the word it will be rendered properly

review-notebook-app · 2024-02-01T07:43:35Z

View / edit / reply to this conversation on ReviewNB

merveenoyan commented on 2024-02-01T07:43:35Z
----------------------------------------------------------------

Same here (for header rendering)

review-notebook-app · 2024-02-01T07:43:36Z

View / edit / reply to this conversation on ReviewNB

merveenoyan commented on 2024-02-01T07:43:35Z
----------------------------------------------------------------

you could do single upload_folder https://huggingface.co/docs/huggingface_hub/guides/upload#upload-a-folder

merveenoyan

thanks a lot @klyap for taking over this! 🦸‍♀️

klyap · 2024-02-04T05:36:07Z

Made formatting changes and also changed the upload step to just upload folder and not individual files!

klyap · 2024-02-13T00:51:12Z

lmk if you have other suggestions, @merveenoyan !

review-notebook-app · 2024-02-14T13:51:56Z

View / edit / reply to this conversation on ReviewNB

MKhalusova commented on 2024-02-14T13:51:55Z
----------------------------------------------------------------

"The below code downloads the zebra2horses dataset and places it in the same folder."

review-notebook-app · 2024-02-14T13:51:57Z

View / edit / reply to this conversation on ReviewNB

MKhalusova commented on 2024-02-14T13:51:56Z
----------------------------------------------------------------

Line #4.    # basic transforms

Is this comment really needed?

review-notebook-app · 2024-02-14T13:51:58Z

View / edit / reply to this conversation on ReviewNB

MKhalusova commented on 2024-02-14T13:51:57Z
----------------------------------------------------------------

Since you explain briefly what a residual block does, maybe also add a sentence or two about these other blocks.

review-notebook-app · 2024-02-14T13:51:59Z

View / edit / reply to this conversation on ReviewNB

MKhalusova commented on 2024-02-14T13:51:59Z
----------------------------------------------------------------

Line #73.    class FeatureMapBlock(nn.Module):

What does this type of block do?

johko · 2024-03-08T12:32:42Z

Hey @klyap, thanks for all the work you put in. It seems like the only thing that actually needs to be adressed now, is loading the dataset from the huggingface hub.
I'll have a look how to make that work

johko · 2024-03-25T21:05:29Z

@klyap and @charchit7 I created a PR to the branch in the repo you have been working on it -> hwaseem04#7
Not sure who has to review that, but it should contain all needed fixes, so we can finally merge this ;)

charchit7 · 2024-03-28T17:13:25Z

Hey @johko looks good to me :)

use HF dataset

johko

Thanks @charchit7 , so from my side this one is finally ready to merge 🚀
Just need one last approval from @merveenoyan I guess

added cycleGAN notebook WIP

f241f50

charchit7 requested review from merveenoyan and MKhalusova as code owners January 10, 2024 05:57

merveenoyan reviewed Jan 10, 2024

View reviewed changes

ATaylorAerospace added the notebook label Jan 15, 2024

sitamgithub-MSIT self-requested a review January 18, 2024 21:58

MKhalusova reviewed Jan 23, 2024

View reviewed changes

Address code review comments

7cc8015

- Using Huggingface hosted dataset and push model to Hub - Use markdown instead of docstring

charchit7 requested review from MKhalusova and merveenoyan January 31, 2024 12:33

merveenoyan reviewed Feb 1, 2024

View reviewed changes

Improve formatting and upload folder instead of 4 files

38d22d1

Move diagram to intro + add more explanations

e5f7cf5

use HF dataset in CycleGAN notebook

c103cc1

Merge pull request #7 from johko/CG_notebook_johko

251c23a

use HF dataset

johko approved these changes Mar 29, 2024

View reviewed changes

johko merged commit 371b18d into huggingface:main Jun 28, 2024
2 checks passed

[WIP] added cycleGAN notebook! #175

[WIP] added cycleGAN notebook! #175

Conversation

charchit7 commented Jan 10, 2024

review-notebook-app bot commented Jan 10, 2024

merveenoyan Jan 10, 2024 • edited Loading

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

merveenoyan Jan 10, 2024 • edited Loading

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

merveenoyan Jan 10, 2024 • edited Loading

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

klyap Feb 1, 2024 • edited Loading

Choose a reason for hiding this comment

merveenoyan Jan 10, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

MKhalusova Feb 14, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

merveenoyan Jan 10, 2024 • edited Loading

Choose a reason for hiding this comment

merveenoyan Jan 10, 2024 • edited Loading

Choose a reason for hiding this comment

merveenoyan Jan 10, 2024 • edited Loading

Choose a reason for hiding this comment

charchit7 commented Jan 10, 2024 • edited Loading

merveenoyan commented Jan 11, 2024

charchit7 commented Jan 11, 2024

sitamgithub-MSIT commented Jan 18, 2024

merveenoyan commented Jan 19, 2024

charchit7 commented Jan 19, 2024

johko commented Jan 19, 2024

MKhalusova Jan 23, 2024 • edited Loading

Choose a reason for hiding this comment

klyap Jan 31, 2024 • edited Loading

Choose a reason for hiding this comment

MKhalusova Jan 23, 2024 • edited Loading

Choose a reason for hiding this comment

MKhalusova Jan 23, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

klyap commented Jan 31, 2024

charchit7 commented Jan 31, 2024

review-notebook-app bot commented Feb 1, 2024 • edited Loading

review-notebook-app bot commented Feb 1, 2024 • edited Loading

review-notebook-app bot commented Feb 1, 2024 • edited Loading

review-notebook-app bot commented Feb 1, 2024 • edited Loading

merveenoyan left a comment

Choose a reason for hiding this comment

klyap commented Feb 4, 2024

klyap commented Feb 13, 2024

review-notebook-app bot commented Feb 14, 2024 • edited Loading

review-notebook-app bot commented Feb 14, 2024 • edited Loading

review-notebook-app bot commented Feb 14, 2024 • edited Loading

review-notebook-app bot commented Feb 14, 2024 • edited Loading

johko commented Mar 8, 2024

johko commented Mar 25, 2024

charchit7 commented Mar 28, 2024

johko left a comment

Choose a reason for hiding this comment

merveenoyan Jan 10, 2024 •

edited

Loading

merveenoyan Jan 10, 2024 •

edited

Loading

merveenoyan Jan 10, 2024 •

edited

Loading

klyap Feb 1, 2024 •

edited

Loading

merveenoyan Jan 10, 2024 •

edited

Loading

MKhalusova Feb 14, 2024 •

edited

Loading

merveenoyan Jan 10, 2024 •

edited

Loading

merveenoyan Jan 10, 2024 •

edited

Loading

merveenoyan Jan 10, 2024 •

edited

Loading

charchit7 commented Jan 10, 2024 •

edited

Loading

MKhalusova Jan 23, 2024 •

edited

Loading

klyap Jan 31, 2024 •

edited

Loading

MKhalusova Jan 23, 2024 •

edited

Loading

MKhalusova Jan 23, 2024 •

edited

Loading

review-notebook-app bot commented Feb 1, 2024 •

edited

Loading

review-notebook-app bot commented Feb 1, 2024 •

edited

Loading

review-notebook-app bot commented Feb 1, 2024 •

edited

Loading

review-notebook-app bot commented Feb 1, 2024 •

edited

Loading

review-notebook-app bot commented Feb 14, 2024 •

edited

Loading

review-notebook-app bot commented Feb 14, 2024 •

edited

Loading

review-notebook-app bot commented Feb 14, 2024 •

edited

Loading

review-notebook-app bot commented Feb 14, 2024 •

edited

Loading