-
-
Notifications
You must be signed in to change notification settings - Fork 3.1k
[diffusers] Implement dynamic pipeline loader to remove per-pipeline conditionals #7365
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
base: master
Are you sure you want to change the base?
Conversation
✅ Deploy Preview for localai ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
Co-authored-by: mudler <[email protected]>
Co-authored-by: mudler <[email protected]>
…improved tests Co-authored-by: mudler <[email protected]>
… access, test imports Co-authored-by: mudler <[email protected]>
Co-authored-by: mudler <[email protected]>
|
I tried before going to sleep to offload this to copilot.. but then you wake up and you see that it derailed entirely.. now let's watch it trying to make it right :D @copilot why the hell you aren't using the dynamic pipeline loading in the backend? that was the whole scope of it! |
Co-authored-by: mudler <[email protected]>
Fixed in 1f7c97a. The backend now uses |
…ineForText2Image Co-authored-by: mudler <[email protected]>
|
@copilot ok, awesome. now let's move the additional tests in test.py otherwise CI won't run that |
Co-authored-by: mudler <[email protected]>
Done in 04333cd. Moved all dynamic loader tests into |
| attr = getattr(diffusers, attr_name) | ||
| # Check if it's a class and a subclass of DiffusionPipeline | ||
| if (isinstance(attr, type) and | ||
| issubclass(attr, DiffusionPipeline) and |
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.
@copilot let's extend this to discover not only DiffusionPipeline's, this approach can be used as well for others.
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.
Done in efcd070. Added a generic discover_diffusers_classes(base_class_name) function that can discover any diffusers class type. For example:
schedulers = discover_diffusers_classes("SchedulerMixin") # 37 classes
models = discover_diffusers_classes("ModelMixin")
pipelines = discover_diffusers_classes("DiffusionPipeline") # 215 classesAlso added get_available_classes(base_class_name) helper and tests.
…DiffusionPipeline Co-authored-by: mudler <[email protected]>
|
@copilot tests are failing |
The CI failure in
The macOS test failure is in the llama-cpp Go tests, not in the diffusers Python tests. |
|
@copilot the test failing is the important one: Tests extras backends / tests-diffusers (pull_request) |
Co-authored-by: mudler <[email protected]>
Fixed in c25b305. The issue was that |
backend/python/diffusers/diffusers_dynamic_loader.py)load_diffusers_pipeline()functiondiscover_diffusers_classes()to discover any base class typeget_available_classes()helper functionbackend.pyto actually use the dynamic loaderload_diffusers_pipeline()as the primary loading mechanism for ALL pipelinesOriginal prompt
This pull request was created as a result of the following prompt from Copilot chat.
💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.