Support for Ldm3d #304
Support for Ldm3d #304
Conversation
There was a problem hiding this comment.
@estelleafl Nice add to the lib 🔥 🚀
I left a few comments, mainly about the doc and the example.
Regarding tests, you can add some here: https://github.com/huggingface/optimum-habana/blob/main/tests/test_diffusers.py
My suggestion is to add only one slow test to check that there is no generation regression such as this one:
optimum-habana/tests/test_diffusers.py
Line 585 in 7e8c855
optimum/habana/diffusers/pipelines/stable_diffusion/pipeline_stable_diffusion_ldm3d.py
Show resolved
Hide resolved
optimum/habana/diffusers/pipelines/stable_diffusion/pipeline_stable_diffusion_ldm3d.py
Outdated
Show resolved
Hide resolved
|
@estelleafl I saw also that the code style check failed, could you run the following from the root of the repo please? pip install black ruff
make style |
Co-authored-by: regisss <[email protected]>
Co-authored-by: regisss <[email protected]>
|
Hi @regisss |
|
Hi @regisss |
|
Hi @estelleafl, thank you for the PR! Regis is off until mid-next week unfortunately. Looking at the workflows, I believe the issue is that these wrokflows run on AWS, and thus require keys stored in github secrets, which may not be used for PRs from external contributors for security reasons: https://securitylab.github.com/research/github-actions-preventing-pwn-requests/. I'll see if implementing https://iterative.ai/blog/testing-external-contributions-using-github-actions-secrets is safe. |
|
Thank you @fxmarty let me know if I can help in any way as well |
|
Hi @estelleafl, could you push to this PR (even an empty commit)? Then I should hopefully be able to trigger the CI. |
|
Hi @fxmarty |
|
The docs for this PR live here. All of your documentation changes will be reflected on that endpoint. |
There was a problem hiding this comment.
@estelleafl Thanks for taking my comments into account!
I left a few more comments. The main one is about removing --ldm3d_model_name_or_path in the example so that we keep only one argument for the model name. I propose an alternative to make sure that the default model is Intel/ldm3d-4c when --ldm3d is given, let me know what you think about it.
Co-authored-by: regisss <[email protected]>
Co-authored-by: regisss <[email protected]>
Co-authored-by: regisss <[email protected]>
regisss
added
run-test
There was a problem hiding this comment.
All tests passed, I'm going to merge! Thanks @estelleafl @PhillipHoward for this nice contribution 🔥 🚀
|
great!! thank you |
|
@regisss could we add ldm3d in here : https://huggingface.co/docs/optimum/habana/index ? |
@estelleafl Yes definitely! I forgot about it but I'm working on a slight lifting of the table where I already included LDM3D: https://github.com/huggingface/optimum-habana/tree/update_model_table#validated-models edit: I linked to the table of the README, but I'm planning to do the same changes to the table of the doc of course |
|
great! thank you @regisss |
|
@estelleafl Here it is: https://github.com/huggingface/optimum-habana#validated-models |
#304) upmerge and resolve conflicts
In this PR we integrated our model LDM3D such as the GaudiStableDiffusion pipeline
Where should I write tests?
Should I create a "https://huggingface.co/docs/optimum/habana/package_reference/stable_diffusion_pipeline_ldm3d"