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

shell scripts in prepare_weights don't work without modification #23

Closed
truedat101 opened this issue May 2, 2024 · 6 comments
Closed

Comments

@truedat101
Copy link
Contributor

truedat101 commented May 2, 2024

Probably there is a difference in the current developer "internal" build vs what got released to OSS. The issues are identified below with solution:

  • git clone https://huggingface.co/runwayml/stable-diffusion-v1-5 ckpts/Base_Model will fail because Base_Model exists. It really should be: git clone https://huggingface.co/runwayml/stable-diffusion-v1-5 ckpts/Base_Model/stable-diffusion-v1-5
  • Without previous change, later when running inference scripts, it will fail because stable-diffusion-v1-5 path is not found. fix is in previous step
  • When re-running these scripts, they will fail because directories exist. TODO: identify a way to "update" ... possibly add update scripts if the models will change.
  • in the prepare_weights/down_magictime_module.sh, you cannot move the repository without first deleting the .git subdirectory inside of MagicTime/.git because the clone repository has a write protected object. Why not use a git submodule? Or simply remove the .git dir in the freshly cloned subdirectory.
  • The mv dir command in prepare_weights/down_magictime_module.sh because the directory exists. Either use cp -r, or simply do mv MagicTime/Magic_Weights/* ckpts/Magic_Weights/
@truedat101
Copy link
Contributor Author

A CI server will catch all of these kinds of things. The easiest way to avoid a CI server is to use a docker container, and just make sure the commands all run and you can launch some test script.

@SHYuanBest
Copy link
Member

Thanks for your interest. Is the problem solved now? We currently don’t have the manpower to package it into docker. This step will be completed in the future.

@truedat101
Copy link
Contributor Author

I can resolve it, but I am pointing out the scripts are failing as currently designed. I can provide a gist with a fix or a pull request if your team will take contributions. Several other members of my team are planning to use the project but I need to make it possible to set up and install.

@SHYuanBest
Copy link
Member

ok, please submit a PR

@truedat101
Copy link
Contributor Author

Will work on this tonight.

@SHYuanBest
Copy link
Member

thanks

truedat101 added a commit to truedat101/MagicTime that referenced this issue May 8, 2024
SHYuanBest added a commit that referenced this issue May 9, 2024
Related to #23, shell scripts should run successfully w/o modification
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

No branches or pull requests

2 participants