[WIP] [Refactor]Add Base Class for Diffusion Pipelines#2811
Draft
alex-jw-brooks wants to merge 11 commits intovllm-project:mainfrom
Draft
[WIP] [Refactor]Add Base Class for Diffusion Pipelines#2811alex-jw-brooks wants to merge 11 commits intovllm-project:mainfrom
alex-jw-brooks wants to merge 11 commits intovllm-project:mainfrom
Conversation
Signed-off-by: Alex Brooks <albrooks@redhat.com>
Signed-off-by: Alex Brooks <albrooks@redhat.com>
Signed-off-by: Alex Brooks <albrooks@redhat.com>
Signed-off-by: Alex Brooks <albrooks@redhat.com>
Signed-off-by: Alex Brooks <albrooks@redhat.com>
Add a track_init_args decorator that wraps __init__ to record which kwargs the caller explicitly passed. merge_with_def_params() checks _init_kwargs to fill in pipeline defaults only for fields the caller never touched, correctly preserving explicitly-set falsy values like 0 or False. _convert_dataclasses_to_dict() in entrypoints/utils.py is updated to use field iteration instead of asdict(), skipping non-init fields and None values matching field defaults. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Alex Brooks <albrooks@redhat.com>
Signed-off-by: Alex Brooks <albrooks@redhat.com>
Signed-off-by: Alex Brooks <albrooks@redhat.com>
Signed-off-by: Alex Brooks <albrooks@redhat.com>
Signed-off-by: Alex Brooks <albrooks@redhat.com>
Signed-off-by: Alex Brooks <albrooks@redhat.com>
f1ef9db to
fa50111
Compare
4 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Purpose
For this RFC: #2189
As a consequence of this refactor, will also fix issues with default params not refreshing properly when
num_inference_stepsisn't set: #2189. If this ends up merged before #2240, we won't need that PR anymore.Current State
VllmDiffusionPipelineVllmDiffusionPipelineinstances have asampling_param_defaultsproperty which resolves to aDiffusionParamOverridesobject, setting default values for sampling params on a per pipeline basisDiffusionParamOverridescan be merged withOmniDiffusionSamplingParamswith the prioritypassed by user > pipeline default (in DiffusionParamOverrides) > OmniDiffusionSamplingParams defaulttrack_init_argsdecorator, which just saves the names of the fields that were passed by the user when initializing the dataclass. This is needed for the following case:fooin theOmniDiffusionSamplingParamsis0, but a pipeline setsfoo=1in itsDiffusionParamOverrides, resolving should give 1fooin the pipeline to set a final value of0. I.e., we need to be able to distinguish between when a user passes a default value inOmniDiffusionSamplingParamsvs when it's set by default.Things Left
DiffusionParamOverridespipe.supports_sequence_parallelcan essentially be boiled down tohasattr(pipe.get_transformer(), "_sp_plan"), etcTest Plan
tests/diffusion/inputs/test_data.py)OmniDiffusionSamplingParamsquickly without loading model weights. (These are intests/diffusion/models/test_base.py)This is joint work with @vraiti , thanks for the help! 🙂
Will move this out of draft once it's cleaned up and tested since it's quite messy atm, but opening the draft PR in case people have comments on the direction. FYI @wtomin @SamitHuang @lishunyang12 @asukaqaq-s @hsliuustc0106 @fhfuih