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 MarigoldPipeline to core #7522

Closed
yiyixuxu opened this issue Mar 29, 2024 · 5 comments · Fixed by #7847
Closed

move MarigoldPipeline to core #7522

yiyixuxu opened this issue Mar 29, 2024 · 5 comments · Fixed by #7847

Comments

@yiyixuxu
Copy link
Collaborator

yiyixuxu commented Mar 29, 2024

Model/Pipeline/Scheduler description

Marigold depth pipeline is one of our most highly used community pipeline (it already has 40k downloads on the hub) - we are going to integrate it into core upon popular demand

Does anyone in the community like to take a stab at this? It is already a community pipeline, so we do not need to convert weights. however, we may need to refactor it so that the implementation follows the diffusers' design philosophy and that the codes are highly readable and easy to tweak. You can read about philosophy here https://huggingface.co/docs/diffusers/en/conceptual/philosophy

Of course, it will be an iterative process! You can start the PR by moving the pipeline code into a folder under src, and tagging diffusers maintainers for an initial review

Provide useful links for the implementation

community pipeline: https://github.com/huggingface/diffusers/blob/main/examples/community/marigold_depth_estimation.py)

original issue: #6533

@toshas
Copy link
Contributor

toshas commented Mar 30, 2024

@markkua and I would like to help shape the way Marigold is integrated. We need to discuss this internally and with you to plan this design carefully and enable the pipeline's extensibility to other modalities (which are brewing). In the meantime, can @yiyixuxu please point us to a PR for a community pipeline promotion to the core so we can use it as a reference? Particularly, I'm curious to see how core philosophy is maintained (1) to balance modularity vs ensuring the quality of operation in a default setting and (2) automated test suite, if any

@markkua
Copy link
Contributor

markkua commented Mar 30, 2024

A PR #7524 to the community pipeline is just created for some small updates. Please also take this update into consideration.

@yiyixuxu
Copy link
Collaborator Author

yiyixuxu commented Mar 30, 2024

hey @toshas and @markkua
For 'MarigoldPipeline`, it seems like it will be pretty straightforward, so it will not require any drastic changes. If no one from the community picks this up, I will work on it. We will definitely ask you to review it.

we don't have an example of community pipeline -> official pipeline that's already merged and can serve as a good reference. Marigold has the chance to become the first to come from the community! two other community pipelines are being moved to official pipelines, feel free to take a look at the PRs; they are both more challenging than MarigoldPipeline: face-id requires a different design when integrated into core; and ControlNet-XS, the original implementation deviates a lot from our style so a lot of works need to be done to refactor it

face-id: #7186
controlnet-xs: #6772

@yiyixuxu
Copy link
Collaborator Author

We need to discuss this internally and with you to plan this design carefully and enable the pipeline's extensibility to other modalities (which are brewing)

do we have a channel with you on slack?

@toshas
Copy link
Contributor

toshas commented Mar 30, 2024

do we have a channel with you on slack?

Yes, hf-marigold-colab, I asked to add you there

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 a pull request may close this issue.

3 participants