-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Added guide for CycleGAN #845
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
Conversation
|
Check out this pull request on See visual diffs & provide feedback on Jupyter Notebooks. Powered by ReviewNB |
|
@sdash77 Please do the first round of reviews. |
| @@ -0,0 +1,128 @@ | |||
| { | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"In arcgis.learn, all the discriminators and generators have been grouped into a single model" Please remove this, it is the inherent architecture of cyclegan. Its grouped everywhere.
Describe a little about the objective function or loss that we get combining the three losses discussed above. feels incomplete.
Reply via ReviewNB
|
looks fine to me @mayank2 Thanks. |
|
Seems fine to me. |
| @@ -0,0 +1,198 @@ | |||
| { | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| @@ -0,0 +1,198 @@ | |||
| { | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"There are some challenges in Pix2Pix is that the images should be paired which are randomly organized or unpaired and we want to convert images from one domain to another just like " -> this sentence seems incomplete. Further, can you elaborate or reword the limitations of Pix2Pix model?
I think we also need to elaborate what you mean by an unpaired dataset
Reply via ReviewNB
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, the sentence is incomplete. Thanks for pointing it out .
AtmaMani
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changes requested via reviewnb
<insert pull request description here>
Checklist
Please go through each entry in the below checklist and mark an 'X' if that condition has been met. Every entry should be marked with an 'X' to be get the Pull Request approved.
imports are in the first cell? First block of imports are standard libraries, second block are 3rd party libraries, third block are allarcgisimports? Note that in some cases, for samples, it is a good idea to keep the imports next to where they are used, particularly for uncommonly used features that we want to highlight.GISobject instantiations are one of the following?gis = GIS()gis = GIS('https://www.arcgis.com', 'arcgis_python', 'P@ssword123')gis = GIS(profile="your_online_profile")gis = GIS('https://pythonapi.playground.esri.com/portal', 'arcgis_python', 'amazing_arcgis_123')gis = GIS(profile="your_enterprise_portal")./misc/setup.pyand/or./misc/teardown.py?<img src="base64str_here">instead of<img src="https://some.url">? All map widgets contain a static image preview? (Callmapview_inst.take_screenshot()to do so)os.path.join()? (Instead ofr"\foo\bar",os.path.join(os.path.sep, "foo", "bar"), etc.)