support lingbot video#1236
Conversation
There was a problem hiding this comment.
Code Review
This pull request adds support for the LingBot-Video model (covering t2i, t2v, and i2v tasks) by introducing configuration files, a Qwen3VL text encoder, VAE modules, custom schedulers, runners, and inference pipelines. The code review identified several critical bugs and performance bottlenecks that should be addressed: duplicate field definitions in the LingBotVideoPreInferOutput dataclass, potential TypeError and AttributeError exceptions due to missing checks on configuration and image path variables, and improper cooperative multiple inheritance in the scheduler. Additionally, several performance optimizations were suggested, such as caching rotary embedding calculations to avoid GPU-to-CPU synchronization, avoiding host-to-device copies when creating sequence length tensors, and pre-casting configuration variables to prevent repeated type conversions in hot paths.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| class LingBotVideoPreInferOutput: | ||
| hidden_states: torch.Tensor | ||
| rotary_emb: torch.Tensor | ||
| temb6: torch.Tensor | ||
| temb_input: torch.Tensor | ||
| n_video: int | ||
| grid_t: int | ||
| grid_h: int | ||
| grid_w: int | ||
| latent_shape: tuple | ||
| temb_input: torch.Tensor | ||
| temb6: torch.Tensor | ||
| rotary_emb: torch.Tensor | ||
| n_video: int | ||
| grid_t: int | ||
| grid_h: int | ||
| grid_w: int | ||
| latent_shape: tuple |
There was a problem hiding this comment.
The LingBotVideoPreInferOutput dataclass contains duplicate field definitions for temb_input, temb6, rotary_emb, n_video, grid_t, grid_h, grid_w, and latent_shape. Removing these duplicate fields improves code clarity and prevents potential issues with static type checkers.
| class LingBotVideoPreInferOutput: | |
| hidden_states: torch.Tensor | |
| rotary_emb: torch.Tensor | |
| temb6: torch.Tensor | |
| temb_input: torch.Tensor | |
| n_video: int | |
| grid_t: int | |
| grid_h: int | |
| grid_w: int | |
| latent_shape: tuple | |
| temb_input: torch.Tensor | |
| temb6: torch.Tensor | |
| rotary_emb: torch.Tensor | |
| n_video: int | |
| grid_t: int | |
| grid_h: int | |
| grid_w: int | |
| latent_shape: tuple | |
| @dataclass | |
| class LingBotVideoPreInferOutput: | |
| hidden_states: torch.Tensor | |
| rotary_emb: torch.Tensor | |
| temb6: torch.Tensor | |
| temb_input: torch.Tensor | |
| n_video: int | |
| grid_t: int | |
| grid_h: int | |
| grid_w: int | |
| latent_shape: tuple |
| class LingBotVideoRotaryEmbedding: | ||
| def __init__(self, axes_dims, axes_lens, theta): | ||
| self.axes_dims = tuple(axes_dims) | ||
| self.axes_lens = list(axes_lens) | ||
| self.theta = theta | ||
| self.freqs_cis = None | ||
|
|
||
| def __call__(self, position_ids): | ||
| device = position_ids.device | ||
| max_vals = position_ids.max(dim=0).values.tolist() | ||
| needs_rebuild = self.freqs_cis is None or any(max_val >= axis_len for max_val, axis_len in zip(max_vals, self.axes_lens)) | ||
| if needs_rebuild: | ||
| for i in range(len(self.axes_lens)): | ||
| if max_vals[i] >= self.axes_lens[i]: | ||
| self.axes_lens[i] = int(max_vals[i] * 1.5) + 1 | ||
| self.freqs_cis = precompute_freqs_cis(self.axes_dims, tuple(self.axes_lens), self.theta) | ||
| self.freqs_cis = [freqs.to(device) for freqs in self.freqs_cis] | ||
| elif self.freqs_cis[0].device != device: | ||
| self.freqs_cis = [freqs.to(device) for freqs in self.freqs_cis] | ||
| return torch.cat([self.freqs_cis[i][position_ids[:, i]] for i in range(len(self.axes_dims))], dim=-1) |
There was a problem hiding this comment.
Calling position_ids.max(dim=0).values.tolist() on every step of the denoising loop causes a GPU-to-CPU synchronization, which can significantly degrade inference performance. Since the sequence length and coordinates are static throughout a single generation run, we can cache the previous sequence length and only perform the rebuild check when the sequence length changes.
class LingBotVideoRotaryEmbedding:
def __init__(self, axes_dims, axes_lens, theta):
self.axes_dims = tuple(axes_dims)
self.axes_lens = list(axes_lens)
self.theta = theta
self.freqs_cis = None
self._prev_seq_len = None
def __call__(self, position_ids):
device = position_ids.device
seq_len = position_ids.shape[0]
needs_rebuild = self.freqs_cis is None or seq_len != self._prev_seq_len
if needs_rebuild:
self._prev_seq_len = seq_len
max_vals = position_ids.max(dim=0).values.tolist()
for i in range(len(self.axes_lens)):
if max_vals[i] >= self.axes_lens[i]:
self.axes_lens[i] = int(max_vals[i] * 1.5) + 1
self.freqs_cis = precompute_freqs_cis(self.axes_dims, tuple(self.axes_lens), self.theta)
self.freqs_cis = [freqs.to(device) for freqs in self.freqs_cis]
elif self.freqs_cis[0].device != device:
self.freqs_cis = [freqs.to(device) for freqs in self.freqs_cis]
return torch.cat([self.freqs_cis[i][position_ids[:, i]] for i in range(len(self.axes_dims))], dim=-1)| raise ImportError("transformers with Qwen3VL support is required for LingBot-Video text encoding.") | ||
| self.config = config | ||
| self.cpu_offload = config.get("qwen3vl_cpu_offload", config.get("text_encoder_cpu_offload", False)) | ||
| self.hidden_state_skip_layer = int(config.get("hidden_state_skip_layer", 0)) |
There was a problem hiding this comment.
Unconditionally casting hidden_state_skip_layer to int will raise a TypeError if it is explicitly configured as None (or null in JSON). It also makes the subsequent self.hidden_state_skip_layer is not None checks dead code. Only cast to int if the value is not None.
| self.hidden_state_skip_layer = int(config.get("hidden_state_skip_layer", 0)) | |
| hidden_state_skip_layer = config.get("hidden_state_skip_layer", 0) | |
| self.hidden_state_skip_layer = int(hidden_state_skip_layer) if hidden_state_skip_layer is not None else None |
| q = apply_rotary_emb(weights.attn.norm_q.apply(q), rotary_emb) | ||
| k = apply_rotary_emb(weights.attn.norm_k.apply(k), rotary_emb) | ||
| seq_len = q.shape[0] | ||
| cu_seqlens = torch.tensor([0, seq_len], dtype=torch.int32, device=q.device) |
There was a problem hiding this comment.
Creating cu_seqlens using torch.tensor([0, seq_len], ...) on every attention block execution causes a host-to-device copy. We can optimize this by creating a zero-initialized tensor and setting the second element, which avoids copying list elements from CPU to GPU.
cu_seqlens = torch.zeros(2, dtype=torch.int32, device=q.device)
cu_seqlens[1] = seq_len|
|
||
| class LingBotVideoScheduler(WanScheduler): | ||
| def __init__(self, config): | ||
| BaseScheduler.__init__(self, config) |
There was a problem hiding this comment.
Calling BaseScheduler.__init__(self, config) directly bypasses the immediate parent class WanScheduler's initialization. Use super().__init__(config) to ensure proper cooperative multiple inheritance and that any initialization in WanScheduler is correctly executed.
| BaseScheduler.__init__(self, config) | |
| super().__init__(config) |
| image_path = self.input_info.image_path.split(",")[0] | ||
| if not image_path: | ||
| raise ValueError("LingBot-Video i2v requires --image_path.") |
There was a problem hiding this comment.
If self.input_info.image_path is None, calling .split(",") will raise an AttributeError. Adding a fallback check ensures robustness against missing or uninitialized image paths.
| image_path = self.input_info.image_path.split(",")[0] | |
| if not image_path: | |
| raise ValueError("LingBot-Video i2v requires --image_path.") | |
| image_path = getattr(self.input_info, "image_path", "") or "" | |
| if not image_path: | |
| raise ValueError("LingBot-Video i2v requires --image_path.") | |
| image_path = image_path.split(",")[0] |
| self.n_group = config.get("n_group", 4) | ||
| self.topk_group = config.get("topk_group", 2) |
There was a problem hiding this comment.
Casting n_group and topk_group to int in __init__ avoids repeated casting inside the performance-critical _group_limited_topk and _route methods.
| self.n_group = config.get("n_group", 4) | |
| self.topk_group = config.get("topk_group", 2) | |
| self.n_group = int(config.get("n_group", 4)) if config.get("n_group") is not None else None | |
| self.topk_group = int(config.get("topk_group", 2)) if config.get("topk_group") is not None else None |
No description provided.