Merged
Conversation
llmc-reviewer
approved these changes
Apr 2, 2026
Contributor
There was a problem hiding this comment.
Code Review
This pull request refactors the target shape assignment logic in the Flux2Klein runner, moving the i2i task shape initialization to the input encoder. However, the changes introduce a NameError in set_target_shape by using multiple_of before its definition. Additionally, the target_shape is not correctly updated when a custom shape is provided, and useful logging was removed.
Comment on lines
256
to
269
| task = self.config.get("task", "t2i") | ||
| if task == "i2i" and hasattr(self.input_info, "auto_width"): | ||
| width = self.input_info.auto_width | ||
| height = self.input_info.auto_height | ||
| else: | ||
| if task == "i2i": # for i2i task, the target shape is already set in _run_input_encoder_local_i2i | ||
| height, width = self.input_info.target_shape | ||
| else: # for t2i task, calculate the target shape based on the resolution | ||
| custom_shape = self.get_custom_shape() | ||
| if custom_shape is not None: | ||
| width, height = custom_shape | ||
| else: | ||
| calculated_width, calculated_height, _ = calculate_dimensions(self.resolution * self.resolution, 16 / 9) | ||
| width = calculated_width // multiple_of * multiple_of | ||
| height = calculated_height // multiple_of * multiple_of | ||
|
|
||
| self.input_info.auto_width = width | ||
| self.input_info.auto_height = height | ||
|
|
||
| self.input_info.target_shape = (height, width) | ||
| logger.info(f"Flux2Klein Image Runner set target shape: {width}x{height}") | ||
| self.input_info.target_shape = (height, width) | ||
|
|
||
| multiple_of = self.config.get("vae_scale_factor", 8) * 2 |
Contributor
There was a problem hiding this comment.
The set_target_shape method contains a critical bug and a logic error:
- NameError:
multiple_ofis used on lines 265 and 266 before it is defined on line 269. This will cause a crash during execution for thet2itask. - Logic Error:
self.input_info.target_shapeis only updated inside theelseblock (line 267). If acustom_shapeis provided (line 261), thetarget_shapeattribute ininput_infois never updated, which will lead to incorrect behavior in subsequent pipeline stages that rely on this value. - Missing Logging: The useful log message indicating the final target shape was removed.
I recommend moving the multiple_of definition to the top and ensuring target_shape is always assigned for the t2i task.
Suggested change
| task = self.config.get("task", "t2i") | |
| if task == "i2i" and hasattr(self.input_info, "auto_width"): | |
| width = self.input_info.auto_width | |
| height = self.input_info.auto_height | |
| else: | |
| if task == "i2i": # for i2i task, the target shape is already set in _run_input_encoder_local_i2i | |
| height, width = self.input_info.target_shape | |
| else: # for t2i task, calculate the target shape based on the resolution | |
| custom_shape = self.get_custom_shape() | |
| if custom_shape is not None: | |
| width, height = custom_shape | |
| else: | |
| calculated_width, calculated_height, _ = calculate_dimensions(self.resolution * self.resolution, 16 / 9) | |
| width = calculated_width // multiple_of * multiple_of | |
| height = calculated_height // multiple_of * multiple_of | |
| self.input_info.auto_width = width | |
| self.input_info.auto_height = height | |
| self.input_info.target_shape = (height, width) | |
| logger.info(f"Flux2Klein Image Runner set target shape: {width}x{height}") | |
| self.input_info.target_shape = (height, width) | |
| multiple_of = self.config.get("vae_scale_factor", 8) * 2 | |
| multiple_of = self.config.get("vae_scale_factor", 8) * 2 | |
| task = self.config.get("task", "t2i") | |
| if task == "i2i": # for i2i task, the target shape is already set in _run_input_encoder_local_i2i | |
| height, width = self.input_info.target_shape | |
| else: # for t2i task, calculate the target shape based on the resolution | |
| custom_shape = self.get_custom_shape() | |
| if custom_shape is not None: | |
| width, height = custom_shape | |
| else: | |
| calculated_width, calculated_height, _ = calculate_dimensions(self.resolution * self.resolution, 16 / 9) | |
| width = calculated_width // multiple_of * multiple_of | |
| height = calculated_height // multiple_of * multiple_of | |
| self.input_info.target_shape = (height, width) | |
| logger.info(f"Flux2Klein Image Runner set target shape: {width}x{height}") |
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.
No description provided.