[Modular] guard ModularPipeline.blocks attribute

[yiyixuxu]

pipeline blocks are definitions - they're dynamically composable. you can add, remove, combine, or swap blocks.
Pipelines are stateful, and modifying the blocks attribute on a pipeline directly won't update the pipeline's internal state correctly.

This change makes blocks a property that returns a deepcopy, so users can't change the blocks and expect it to work. e.g. if you do something like this, the original pipeline would still work (currently it does not)

pipe = ModularPipeline.from_pretrained()
text_block = pipe.blocks.sub_blocks.pop("text_encoder")

[HuggingFaceDocBuilderDev]

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.

[sayakpaul]

Important fix! Let's also add a mini test case for this?

[DN6]

Just seeking some clarification. In this PR we want to enforce the fact that blocks cannot/should not be modified after the pipeline is initialized? If we want to customize the blocks in a pipeline, it should happen before the pipeline is created? Am I understanding that correctly?

If that is the intent, the change is good with me. Something to consider about the provided example pipe.blocks.sub_blocks.pop("text_encoder"); a pop method typically implies that an element has been removed from a collection. So a user trying this might expect the block to be gone. And now it won't be.

[yiyixuxu]

@DN6

In this PR we want to enforce the fact that blocks cannot/should not be modified after the pipeline is initialized? If we want to customize the blocks in a pipeline, it should happen before the pipeline is created?

Yes, that's exactly right! Or alternatively, users can "take" the blocks from the pipeline, modify them, and create a new pipeline.

I also have this workflow PR that I'm working on (#13028), from there importing the pipeline is all users need - they can extract different block presets from it programmatically with get_workflow(text2image, image2image, etc.).

The recommended usage I'm thinking of is to treat XXModularPipeline like a container:

# load the pipeline
loader = ModularPipeline.from_pretrained("Qwen/Qwen-Image")

# take the blocks from it to make customizations
new_blocks = loader.blocks
## or if want to preset without the autoblocks 
# text2image_blocks = loader.get_workflow("text2image")
new_blocks.pop("text_encoder")

# create the new pipeline
pipe = new_blocks.to_pipeline()
# reuse existing components if we want to load the loader 
pipe.update_components(text_encoder=loader.text_encoder, ...) 
pipe.load_components(...)  # load new components etc

Something to consider about the provided example pipe.blocks.sub_blocks.pop("text_encoder"); a pop method typically implies that an element has been removed from a collection. So a user trying this might expect the block to be gone. And now it won't be.

I have exactly the same concern! My current thinking is to add a note in the documentation that modifying blocks after the pipeline is created is not supported, since the pipeline state does not automatically update based on changes to blocks.

I thought about adding a warning when users access .blocks, but it would be very annoying since you will get a warn even just to read it

do you think the documentation is enought? another option I'm thinking is to rename the propeerty to a get_blocks methods so that it's clear you are getting a copy (can combine with the get_workflow too e.g. get_blocks(workflow=...)

let me know your thoughts!!!

[DN6]

I thought about adding a warning when users access .blocks, but it would be very annoying since you will get a warn even just to read it

No I think that might be overkill 😅

Is this a valid swapping workflow?

# load the pipeline
pipe = ModularPipeline.from_pretrained("Qwen/Qwen-Image")

# take the blocks from it to make customizations
new_blocks = loader.blocks
# remove text encoder step
new_blocks.pop("text_encoder")

my_custom_block = ModularPipelineBlocks.from_pretrained("<...>", trust_remote_code=True)
new_blocks.insert("text_encoder", my_custom_block, 1)

# create the new pipeline
new_pipe = new_blocks.to_pipeline() # can we reuse init_pipeline? 

# reuse existing components if we want to load the loader 
new_pipe.update_components(text_encoder=loader.text_encoder, ...) 
new_pipe.load_components(...)  # load new components etc

[yiyixuxu]

@DN6
ohh yeah should be valid
the to_pipeline was a typo from my example. we only have init_pipeline