Conversation
Summary of ChangesHello @Lihuang-a, 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 enhances the Wan2.1 model ecosystem by integrating new distilled model configurations and providing comprehensive documentation. The changes enable users to leverage FP8 quantization and Lora adaptations for improved performance and flexibility in image-to-video and text-to-video generation, with detailed performance metrics now available in the updated beginner's guide. 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. Changelog
Activity
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
|
There was a problem hiding this comment.
Code Review
This pull request adds new configurations, scripts, and documentation for wan21 models, introducing new inference options. My review focuses on improving maintainability and correctness. I've identified several instances of hardcoded absolute paths in new configuration files and shell scripts that should be made more portable. I also found a critical configuration mismatch in one of the new scripts regarding GPU settings and torchrun usage. Additionally, there are some minor typos and structural issues in the updated documentation that I've suggested corrections for.
| export CUDA_VISIBLE_DEVICES=0 | ||
|
|
||
| # set environment variables | ||
| source ${lightx2v_path}/scripts/base/base.sh | ||
|
|
||
| torchrun --nproc_per_node=8 -m lightx2v.infer \ |
There was a problem hiding this comment.
There is a contradiction in the script's configuration for parallelism. export CUDA_VISIBLE_DEVICES=0 restricts execution to a single GPU, but torchrun --nproc_per_node=8 attempts to launch 8 processes for multi-GPU execution. If this script is intended for single-GPU use, torchrun is not necessary and python -m lightx2v.infer ... should be used instead. If it's for multi-GPU, CUDA_VISIBLE_DEVICES should be set to allow 8 GPUs (e.g., 0,1,2,3,4,5,6,7).
| 注意:蒸馏模型的model_path为原模型的路径,在 config_json 中填入蒸馏模型路径 | ||
| 使用8张H100,运行时间及使用`watch -n 1 nvidia-smi`观测的峰值显存参考如下: | ||
| 1. Wan2.1-I2V-14B-480P模型:Total Cost cost 131.553567 seconds;44624MiB | ||
| 2. 步数蒸馏模型 Lora:Total Total Cost cost 38.337339 seconds;43850MiB |
| "offload_granularity": "model", | ||
| "denoising_step_list": [1000, 750, 500, 250], | ||
| "dit_quantized": true, | ||
| "dit_quantized_ckpt": "/models/Wan-AI/Wan2.1-Distill-Models/wan2.1_t2v_14b_scaled_fp8_e4m3_lightx2v_4step.safetensors", |
There was a problem hiding this comment.
The configuration file contains a hardcoded absolute path for the model checkpoint: /models/Wan-AI/Wan2.1-Distill-Models/wan2.1_t2v_14b_scaled_fp8_e4m3_lightx2v_4step.safetensors. This harms portability. Consider using a relative path, a placeholder, or a mechanism to resolve paths at runtime based on user configuration.
| "denoising_step_list": [1000, 750, 500, 250], | ||
| "lora_configs": [ | ||
| { | ||
| "path": "/models/Wan-AI/Wan2.1-Distill-Loras/wan2.1_t2v_14b_lora_rank64_lightx2v_4step.safetensors", |
There was a problem hiding this comment.
The configuration file contains a hardcoded absolute path for the model: /models/Wan-AI/Wan2.1-Distill-Loras/wan2.1_t2v_14b_lora_rank64_lightx2v_4step.safetensors. This harms portability. Consider using a relative path, a placeholder, or a mechanism to resolve paths at runtime based on user configuration.
| "cpu_offload": false, | ||
| "offload_granularity": "model", | ||
| "denoising_step_list": [1000, 750, 500, 250], | ||
| "dit_original_ckpt": "/models/Wan-AI/Wan2.1-Distill-Models/wan2.1_t2v_14b_lightx2v_4step.safetensors" |
There was a problem hiding this comment.
The configuration file contains a hardcoded absolute path for the model checkpoint: /models/Wan-AI/Wan2.1-Distill-Models/wan2.1_t2v_14b_lightx2v_4step.safetensors. This harms portability. Consider using a relative path, a placeholder, or a mechanism to resolve paths at runtime based on user configuration.
| lightx2v_path=/data1/yongyang/huangli/tap/LightX2V | ||
| model_path=/models/Wan-AI/Wan2.1-I2V-14B-480P |
There was a problem hiding this comment.
This script contains hardcoded, developer-specific absolute paths for lightx2v_path and model_path. This is not portable and requires every user to edit the file. For consistency with other scripts and better user experience, please remove the hardcoded values. The user can then set them as environment variables before running the script, which is already checked by scripts/base/base.sh.
| lightx2v_path=/data1/yongyang/huangli/tap/LightX2V | |
| model_path=/models/Wan-AI/Wan2.1-I2V-14B-480P | |
| lightx2v_path= | |
| model_path= |
| lightx2v_path=/data1/yongyang/huangli/tap/LightX2V | ||
| model_path=/models/Wan-AI/Wan2.1-I2V-14B-480P |
There was a problem hiding this comment.
This script contains hardcoded, developer-specific absolute paths for lightx2v_path and model_path. This is not portable and requires every user to edit the file. For consistency with other scripts and better user experience, please remove the hardcoded values. The user can then set them as environment variables before running the script, which is already checked by scripts/base/base.sh.
| lightx2v_path=/data1/yongyang/huangli/tap/LightX2V | |
| model_path=/models/Wan-AI/Wan2.1-I2V-14B-480P | |
| lightx2v_path= | |
| model_path= |
| "cpu_offload": true, | ||
| "offload_granularity": "model", | ||
| "denoising_step_list": [1000, 750, 500, 250], | ||
| "dit_original_ckpt": "/models/Wan-AI/Wan2.1-Distill-Models/wan2.1_i2v_480p_int8_lightx2v_4step.safetensors" |
There was a problem hiding this comment.
The configuration file contains a hardcoded absolute path for the model checkpoint: /models/Wan-AI/Wan2.1-Distill-Models/wan2.1_i2v_480p_int8_lightx2v_4step.safetensors. This harms portability. Consider using a relative path, a placeholder, or a mechanism to resolve paths at runtime based on user configuration.
| lightx2v_path=/data1/yongyang/huangli/tap/LightX2V | ||
| model_path=/models/Wan-AI/Wan2.1-T2V-14B |
There was a problem hiding this comment.
This script contains hardcoded, developer-specific absolute paths for lightx2v_path and model_path. This is not portable and requires every user to edit the file. For consistency with other scripts and better user experience, please remove the hardcoded values. The user can then set them as environment variables before running the script, which is already checked by scripts/base/base.sh.
| lightx2v_path=/data1/yongyang/huangli/tap/LightX2V | |
| model_path=/models/Wan-AI/Wan2.1-T2V-14B | |
| lightx2v_path= | |
| model_path= |
| lightx2v_path=/data1/yongyang/huangli/tap/LightX2V | ||
| model_path=/models/Wan-AI/Wan2.1-T2V-14B |
There was a problem hiding this comment.
This script contains hardcoded, developer-specific absolute paths for lightx2v_path and model_path. This is not portable and requires every user to edit the file. For consistency with other scripts and better user experience, please remove the hardcoded values. The user can then set them as environment variables before running the script, which is already checked by scripts/base/base.sh.
| lightx2v_path=/data1/yongyang/huangli/tap/LightX2V | |
| model_path=/models/Wan-AI/Wan2.1-T2V-14B | |
| lightx2v_path= | |
| model_path= |
update wan21 doc