-
Notifications
You must be signed in to change notification settings - Fork 30
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
[Model Support] FLUX.1-dev #28
Conversation
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.
Thanks @raoulritter! Did you test FLUX.1-schnell
after your changes?
@@ -0,0 +1,123 @@ | |||
import mlx.core as mx |
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.
Could you please follow the UnitTest.TestCase
usage and make this a simple unit test like this? Also, no need to upload to hub from within the test 👍
Hey @atiorh, I've made the changes as requested. Just unsure in regards to what you exactly expect from the test-conversion-mlx? Should it be moved to the tests or should it maybe even be deleted as it's not really needed. Also checking the merge conflicts it appears these are related to the hosting of the checkpoint. Assuming your team will host these checkpoints yourself so won't resolve these for you. If there is anything else let me know. |
@raoulritter Thanks for the quick iteration! We will revise the testing files, download the ckpt and host it alongside schnell. cc: @arda-argmax |
Hey @raoulritter, I have encountered with problems while trying to run your code. I think you need to make your huggingface checkpoint repo public because I cannot access it. Also, it seems that your branch diverged from our main branch. Can you also rebase it? 🙏 This is the cli command I'm trying to run:
|
Hey @arda-argmax, I think it would perhaps be better based on the conflicts if you guys fixed them according to how you want them instead of me rebasing.
I previously wrote the following
Let me know if you still want me to take a look but perhaps would make more sense from your side. |
I see, sorry I missed it. Can you also share how you run the dev model? I'm getting this error when I'm trying to run your branch with the following command:
Error:
|
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.
Thanks for contributing, this will be really cool! Tests still need some work to conform to the rest of the repo's style, but added some suggestions to use the argmax HF repo. If we want to follow up with tests in another PR, you may want to just confirm the pipeline is working with the new repo, and take out the current tests for later.
Also updated the merge conflicts 👍 Looks like just a linter issue now |
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.
Everything seems to be working 👍 LGTM!
@raoulritter Thank you for your contribution and @ZachNagengast thank you for the pipeline fixes 🙏
Trying to run from this branch and I am getting: Repository Not Found for url: https://huggingface.co/argmaxinc/mlx-FLUX.1-dev/resolve/main/flux1-dev.safetensors. |
@mgierschdev Thanks for the report, there were some restrictions on the model repo that I've removed now, could you try it again and lmk how it goes? You may still need to accept the terms of the license from black forest labs to use it. |
@raoulritter This is an awesome contribution, thank you! |
Hi @ZachNagengast happy to help thanks for fixing the conflicts and nits. I would love to (help) add lora capabilities as well not sure what your team's plan is. |
No description provided.