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 --output_folder, updated params, and tidied documentation #141

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

TaylorBurnham
Copy link

I've been tinkering with this and made the following changes

  • Tables have been added to README.md to tighten up the pages and reduce scrolling.
  • Removed screenshots for Windows and substituted with the commands to make it easier for people to follow.
  • Installation instructions have been adjusted for Windows and filled out a bit more for both operating systems, including a CUDA device compatibility check.
  • Updated the usage list with the latest parameters from the most recent build.
  • Added --output_folder to the command line arguments, which will create the path recursively and return a POSIX path for output.

I really dig this project although depending on what you feed it the results are nightmare fuel.

Added tables to tighten up documentation to reduce scrolling, cuts down
on the amount of empty space between samples.

Updated CLI with the latest instructions from running imagine --help and
included the currently available models and optimizers.

Removed screenshots from the Windows section and provided samples from
running it on my workstation to simplify instructions for newer users.

Added steps to verify CUDA devices are available to torch prior to
running.

Tested on Windows 10 / Python 3.9 / RTX 2070 and AWS Deep Learning AMI
Ubuntu 18.04 / Python 3.6 / Tesla T4
This will create the directory structure recursively and return a POSIX
compatible Path() from Pathlib. Tested on Windows 10 and Ubuntu.
output_folder needs to be explicitly typed otherwise a passed integer
will cause it to fail. Probably has to do with the use of sys.argv and
could be set in cli.py or switch over to argparse, but that is up to
lucidrains if that route is taken.
@TaylorBurnham
Copy link
Author

It looks like cli.py is treating even quoted integers as ints so if the parameter --output_folder=1 was passed it would fail to create the path. I've explicitly typed it on the init() function but cutting over to argparse or something else can help this.

Fixed the out of alignment table in the intro.

Changed a few sentences to be less wordy.
@hwayne
Copy link

hwayne commented Jul 9, 2021

Oh man, this is so useful to me. Thank you!

@batmanscode
Copy link

The output_folder is super useful! Thank you! 😃

Copy link

@gandie gandie left a comment

Choose a reason for hiding this comment

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

mighty fine, lgtm. @lucidrains , merge? 🚀

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.

4 participants