support Wan2.2-VACE-Fun-A14B #844
Conversation
Summary of ChangesHello @gushiqiao, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces comprehensive support for the Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request adds support for the Wan2.2-VACE-Fun-A14B model, including new configuration files, a new runner class Wan22MoeVaceRunner, and example run scripts. The changes also include several correctness and robustness improvements, such as better handling of missing weights, ensuring proper state initialization during inference, and adding support for sequence parallelism in VACE models.
My review focuses on a few key areas:
- I've identified a critical issue in the newly added shell scripts where a trailing backslash will cause them to fail.
- I've also pointed out an opportunity to refactor duplicated code for model path resolution to improve maintainability.
Overall, the changes are well-structured and add significant new functionality. Addressing the identified issues will improve the quality and robustness of the code.
| --negative_prompt "色调艳丽，过曝，静态，细节模糊不清，字幕，风格，作品，画作，画面，静止，整体发灰，最差质量，低质量，JPEG压缩残留，丑陋的，残缺的，多余的手指，画得不好的手部，画得不好的脸部，畸形的，毁容的，形态畸形的肢体，手指融合，静止不动的画面，杂乱的背景，三条腿，背景人很多，倒着走" \ | ||
| --src_video /path/to/post+depth.mp4 \ | ||
| --src_ref_images /path/to/image.png \ | ||
| --save_result_path ${lightx2v_path}/save_results/output_lightx2v_wan22_moe_vace.mp4\ |
There was a problem hiding this comment.
There is a trailing backslash (\) at the end of this line. In a shell script, this escapes the newline character, causing the shell to expect more of the command on the next line. Since there is no next line for this command, it will likely result in a syntax error or hang, preventing the script from running correctly. Please remove the trailing backslash.
| --save_result_path ${lightx2v_path}/save_results/output_lightx2v_wan22_moe_vace.mp4\ | |
| --save_result_path ${lightx2v_path}/save_results/output_lightx2v_wan22_moe_vace.mp4 |
| --negative_prompt "色调艳丽，过曝，静态，细节模糊不清，字幕，风格，作品，画作，画面，静止，整体发灰，最差质量，低质量，JPEG压缩残留，丑陋的，残缺的，多余的手指，画得不好的手部，画得不好的脸部，畸形的，毁容的，形态畸形的肢体，手指融合，静止不动的画面，杂乱的背景，三条腿，背景人很多，倒着走" \ | ||
| --src_video /path/to/post+depth.mp4 \ | ||
| --src_ref_images /path/to/image.png \ | ||
| --save_result_path ${lightx2v_path}/save_results/output_lightx2v_wan22_moe_vace_cfg_parallel.mp4\ |
There was a problem hiding this comment.
Similar to the other script, there is a trailing backslash (\) at the end of this line. This will cause a syntax error because it escapes the newline and the shell expects more input for the torchrun command. Please remove the trailing backslash to fix the script.
| --save_result_path ${lightx2v_path}/save_results/output_lightx2v_wan22_moe_vace_cfg_parallel.mp4\ | |
| --save_result_path ${lightx2v_path}/save_results/output_lightx2v_wan22_moe_vace_cfg_parallel.mp4 |
| super().__init__(config) | ||
| if self.config.get("dit_quantized", False) and self.config.get("high_noise_quantized_ckpt", None): | ||
| self.high_noise_model_path = self.config["high_noise_quantized_ckpt"] | ||
| elif self.config.get("high_noise_original_ckpt", None): | ||
| self.high_noise_model_path = self.config["high_noise_original_ckpt"] | ||
| else: | ||
| self.high_noise_model_path = os.path.join(self.config["model_path"], "high_noise_model") | ||
| if not os.path.isdir(self.high_noise_model_path): | ||
| raise FileNotFoundError(f"High Noise Model does not find") | ||
|
|
||
| if self.config.get("dit_quantized", False) and self.config.get("low_noise_quantized_ckpt", None): | ||
| self.low_noise_model_path = self.config["low_noise_quantized_ckpt"] | ||
| elif not self.config.get("dit_quantized", False) and self.config.get("low_noise_original_ckpt", None): | ||
| self.low_noise_model_path = self.config["low_noise_original_ckpt"] | ||
| else: | ||
| self.low_noise_model_path = os.path.join(self.config["model_path"], "low_noise_model") | ||
| if not os.path.isdir(self.low_noise_model_path): | ||
| raise FileNotFoundError(f"Low Noise Model does not find") |
There was a problem hiding this comment.
The logic for resolving high and low noise model paths in this __init__ method is identical to the one in Wan22MoeRunner. This code duplication can lead to maintenance issues.
To improve this, you could extract the common logic into a mixin class within lightx2v/models/runners/wan/wan_runner.py and have both Wan22MoeRunner and Wan22MoeVaceRunner inherit from it.
Example of a mixin in wan_runner.py:
class Wan22MoePathResolverMixin:
def _resolve_moe_paths(self):
if self.config.get("dit_quantized", False) and self.config.get("high_noise_quantized_ckpt", None):
self.high_noise_model_path = self.config["high_noise_quantized_ckpt"]
# ... rest of the logic for high_noise_model_path
# ... and for low_noise_model_pathThen, you could update Wan22MoeRunner and Wan22MoeVaceRunner to use it:
# In wan_runner.py
class Wan22MoeRunner(WanRunner, Wan22MoePathResolverMixin):
def __init__(self, config):
super().__init__(config)
self._resolve_moe_paths()
# In wan_vace_runner.py
class Wan22MoeVaceRunner(WanVaceRunner, Wan22MoePathResolverMixin):
def __init__(self, config):
super().__init__(config)
self._resolve_moe_paths()This would centralize the path resolution logic, making it easier to maintain.
No description provided.