Fix InputParam required flag silently ignored when a non-None default is set - #14402
Open
Sravan1011 wants to merge 1 commit into
Open
Fix InputParam required flag silently ignored when a non-None default is set#14402Sravan1011 wants to merge 1 commit into
Sravan1011 wants to merge 1 commit into
Conversation
… is set
get_block_state() substituted input_param.default before checking
input_param.required, so a required input with a non-None default
(easy to hit via InputParam.template("x", required=True) since the
template's default carries over) never raised when omitted. Move the
required check ahead of the default substitution so it always fires
against the raw value passed by the caller.
Fixes huggingface#14388
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.
Summary
required=Trueis silently ignored when anInputParamcarries a default #14388:ModularPipelineBlocks.get_block_state()substitutedinput_param.defaultbefore checkinginput_param.required, so a required input with a non-Nonedefault never raised when omitted.InputParam.template("x", required=True), since the template's own default (e.g.num_inference_stepsdefaults to50) carries over unless explicitly overridden withdefault=None.requiredcheck ahead of the default substitution inget_block_state, so it always tests the raw value passed by the caller rather than the value after the fallback default has been applied.Note for maintainers
Several existing blocks declare
InputParam.template(..., required=True)for templates that carry a non-Nonedefault (e.g.num_inference_stepsinkrea2,cosmos,minimax_h3,qwenimage,ltx;batch_sizeanddtypeinltx). With this fix, those inputs now actually enforcerequired=Trueat runtime (previously they silently fell back to the template default). This is the correct behavior per the reported bug, but flagging it here in case any of those sites intended the default to be the real fallback rather thanrequired=True. Happy to follow up with a stricter guard (e.g. rejectingrequired=Trueplus a non-Nonedefault inInputParam.__post_init__) plus a pass over those sites if maintainers prefer that direction -- filing this narrower fix first since it corrects the actual runtime bug with minimal risk.Test plan
InputParam.template("num_inference_steps", required=True)now raises when omitted, and still accepts an explicitly passed value)