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 a very simple GUI using Kivy. #35

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

VincentGourbin
Copy link

A very simple GUI written in Kivy with a local prompt history function
Need to implement a callback for the GUI in the generate.py file.

@anthonywu
Copy link
Collaborator

anthonywu commented Sep 16, 2024

driving by as a user/contributor, deferring to @filipstrand

here's my take:

  1. using kivy is a non-consensus pick given the popularity of gradio in the ML community today
  2. this PR introduces lots of non-core deps to the requirements list - changes like these should be isolated into optional dependencies - which would be something like this in pyproject.toml
[project.optional-dependencies]
kivy_gui = [ ... your list ... ]

and users would opt into the extra deps by pip install mflux[kivy_gui]

iff @filipstrand considers opinionated frontends as something worth including in this repo, then I can see something like:

[project.optional-dependencies]
kivy_gui = [ ... your list ... ]
streamlit_gui = [ ... ]
gradio_gui = [ ... ]

and gui code to be isolated into mflux.ui.[gradio,streamlit,kivy] packages for users to opt into

zooming out though, IMO best practice is to create a separate mflux-kivy repo as a community project and just depend on mflux

@filipstrand
Copy link
Owner

I have only briefly used some frontend image-generation applications myself so my knowledge here is pretty limited, but as far as I have heard Gradio is pretty popular as @anthonywu pointed out. If I remember correctly the official Flux repo had some kind of Streamlit support, but I could not get that repo fully working so I never got to try it out.

Initially I thought it would be nice to support various frontends for MFLUX, but time is very limited and up until now - and for more weeks to come - it will be spent on building out the backend and core-logic.

With this in mind I agree with Anthony and would promote the idea of an external repo/project focused on building a really nice frontend with mflux as a dependency. For example, there already exists some initial drafts of a Gradio GUI here #15.

@anthonywu
Copy link
Collaborator

anthonywu commented Sep 21, 2024

I plan to do a separate repo called mflux-prompt-tools where there will be a fancy CLI based on TBD https://github.com/prompt-toolkit/python-prompt-toolkit or https://www.textualize.io. It'll be terminal-based but will have really fancy (and opinionated) features that I would not burden @filipstrand to adopt.

After that, I am considering a Marimo interactive notebook experience that should be competitive with Gradio in terms of usability as a web-app. Marimo has a way of defining its own dependencies within the notebook file, read here: https://simonwillison.net/2024/Sep/17/serializing-package-requirements-in-marimo-notebooks/ - one of the better emerging frontend experiences.

As for gradio support within the official mflux repo - I can see that as a fair addition, but definitely an optional dependency via pip install mflux[gui] or some opt-in, should not be part of default install set.

@VincentGourbin
Copy link
Author

Totally understand your points, it should be better on separate repos.
Can we just keep the callback function in order for the GUI to estimate and display the running time and expected ETA ?
For the records, I choose, in first instance, to do it with Kivy in order to get a very local solution (that can also keep my prompt history in a local database)

@@ -103,6 +103,9 @@ def generate_image(self, seed: int, prompt: str, config: Config = Config()) -> I

# Evaluate to enable progress tracking
mx.eval(latents)
# Call the progress callback if provided
if progress_callback:
progress_callback(t + 1, config.num_inference_steps)
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think this is worth implementing as an interface for all GUIs to hook into. Another consideration can expect progress_callback callable to return a bool to indicate Continue (True) or Abort (False) so that the UI can stop generate_image.

Should be easy to re-cut a branch from newest main and just implement this.

@anthonywu
Copy link
Collaborator

@filipstrand - I nominate this PR to be closed due to its stale state and also precedence of related projects hosting in their own repos.

@VincentGourbin VincentGourbin marked this pull request as draft November 25, 2024 20:05
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.

3 participants