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

Move model download from the Docker image build step to the run script #8

Closed
notocwilson opened this issue Aug 16, 2023 · 2 comments
Closed
Labels
enhancement New feature or request good first issue Good for newcomers help wanted Extra attention is needed

Comments

@notocwilson
Copy link

With the Docker files, as designed currently, you are staging a rather large file inside of a Docker image, which arguably violates the Docker Image best practice guideline of Don't install unnecessary packages. Ultimately, this dramatically inflates the size of the image to contain the model when it should be treated as a static asset that should be attached to a container.

By migrating the code to download the model to the run script, operators can now attach a volume to /models with storage that is defined outside of the context of the container. This could be a volume that lives on the same host, just as it did before if it was in the context of the Image, or could be provided by any other storage driver supported by Docker. Changes to the Docker Image will not require re-downloading and re-staging the model, significantly lowering both build time and bandwidth required to rebuild the image.

In summary:

  • Remove downloading the model from the Dockerfile for the API service.
  • Update the API service in the compose files to mount a Docker Volume to /models
  • Enhance run.sh with a simple model download manager to check for the existence of a model, and if it does not exist, download it
  • Optional: Expose the model to run as an environment variable, simplifying the amount of Dockerfiles needed to support the project. Have the run.sh model download manager use the environment variable as input to specify what model to check, optionally run, and launch with.
@mayankchhabra
Copy link
Member

Completely agree with you here, @notocwilson! Thanks for outlining your thoughts.

The main reason we went with the model image inside the Docker image was purely because of time constraints. We wanted to launch LlamaGPT as an Umbrel app, and because Umbrel users are mostly non-technical and install it from the app store, they won't be able to see the download progress of the model unless we make changes to the front-end showing the model download progress, and starting the API container once the it is downloaded.

The goal is to:

  • Remove models from Docker images.
  • Update the front-end to list and recommend available models depending upon the RAM size.
  • Show model download progress in the front-end.
  • Allow switching models from the front-end.
  • Start/restart the API container on completition of the model download/change.

@mayankchhabra mayankchhabra added enhancement New feature or request good first issue Good for newcomers help wanted Extra attention is needed labels Aug 16, 2023
@edgar971
Copy link
Contributor

Need help with this? Happy to take this one :).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request good first issue Good for newcomers help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

3 participants