Cosmos3 ModularPipeline#14110
Conversation
yiyixuxu
left a comment
There was a problem hiding this comment.
thanks for working on this!
I did an initial review - I mainly focus on encoder/decoder blocks for now. In modular, these blocks are meant to be run standalone ( e.g. an user encode an image once, keep the latent and reuse them across generations), or combined into a pipeline you can run end-to-end like a standard pipeline.
i will do another pass soon, let me know if you have any questions!
|
Hi @yzhautouskay, thanks for the PR! It does not appear to link an issue it fixes. If this PR addresses an existing issue, please add a closing keyword (e.g. |
|
@yiyixuxu thanks a lot for the review! |
yiyixuxu
left a comment
There was a problem hiding this comment.
thanks!
I did a complete pass this time and left a bit more comments:)
…ice and model dtype
yiyixuxu
left a comment
There was a problem hiding this comment.
thanks for working on the feedbacks so quickly! really nice work!
i left a bit more comments but I think we are close to get this merged
one thing I noticed has not addressed yet is the comment about spliting prepare_latents/packing/denoise step by modality - are you still planning on working on this, or did you run into any issue?
Just want to explain the motivation a bit more: we want to structure the blocks so that:
(1) building a new feature does not require modifying the existing one. when you add a new feature, e.g. enable_sounds, you should be able to just insert new blocks or add a new modality as a sub-block in an conditional block, instead of modifying existing blocks
(2) each workflow is self-contained: a user try to understand video2video should not have to reason about action or sound code path at all, this is also what makes get_workflow("video2video")` meaningful
that said, if you'd prefer to keep it as it as and revist them later when you integrate the new blocks, we're ok with that
| if modular_config_dict is not None: | ||
| pipeline_class = _get_pipeline_class(cls, config=modular_config_dict) | ||
| elif config_dict is not None: | ||
| from diffusers.pipelines.auto_pipeline import _get_model |
There was a problem hiding this comment.
ohh I think you would not need to make change to ModularPipeline.from_pretrained here
if just need to register cosmos3 to auto_pipeline.py https://github.com/huggingface/diffusers/blob/main/src/diffusers/pipelines/auto_pipeline.py#L316
can you revert change in this file and add cosmos3 to auto_pipeline.py? it does not fit into any of the existing task I think, so maybe we can create a AUTO_CONDITION2VIDEO_PIPELINES_MAPPING and then also add it to SUPPORTED_TASKS_MAPPING -> this way _get_model should be able to resolve the model name cosmos3-omni and it should be able to map the default modular blocks from model_index.json
There was a problem hiding this comment.
One blocker: the current Cosmos3 checkpoints declare _class_name: "Cosmos3OmniDiffusersPipeline", while Diffusers exports Cosmos3OmniPipeline. _get_pipeline_class resolves that class name before _get_model,
so the new mapping is not reached with the existing checkpoint metadata.
We’d prefer not to change the checkpoint. Would you be okay with exporting Cosmos3OmniDiffusersPipeline as a compatibility alias of Cosmos3OmniPipeline, then registering Cosmos3 in the new condition-to-video
mapping?
Does that direction sound good?
There was a problem hiding this comment.
is anyone else using that field other than us?
looks like a mistake should have fixed, no?
the current Cosmos3 checkpoints declare _class_name: "Cosmos3OmniDiffusersPipeline", while Diffusers exports Cosmos3OmniPipeline
yiyixuxu
left a comment
There was a problem hiding this comment.
thanks! very nice work
I left some more comments
yiyixuxu
left a comment
There was a problem hiding this comment.
thanks!
very very nice, left some final comments, we can merge this soon
|
The docs for this PR live here. All of your documentation changes will be reflected on that endpoint. The docs are available until 30 days after the last update. |
What does this PR do?
Summary
Cosmos3OmniModularPipelineandCosmos3OmniBlocks.docs/source/en/api/pipelines/cosmos3.md.Test Plan
PYTHONPATH=src python -m pytest -q tests/pipelines/cosmos/test_cosmos3_modular_parity.py -vvBefore submitting
.ai/review-rules.md?documentation guidelines, and
here are tips on formatting docstrings.
Who can review?
Anyone in the community is free to review the PR once the tests have passed. Feel free to tag
members/contributors who may be interested in your PR.