update seko ar rope#1222
Conversation
![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 updates configurations and scripts to use new model paths, adjusts parallel sequence size, and configures AR settings. It also introduces support for sink tokens and global end offsets in the RoPE cache logic in both PyTorch and Triton implementations. The review feedback highlights potential division-by-zero errors in both PyTorch and Triton when local_per_frame is zero, suggests avoiding tl.maximum on compile-time scalars to prevent Triton compilation issues, and recommends replacing hardcoded absolute paths in configuration files and scripts with portable 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.
| ref_frames = ref_tokens // local_per_frame | ||
| ref_frame_idx = position_idx // local_per_frame | ||
| gen_frame_idx = ref_frames + gen_idx // local_per_frame | ||
| frame_idx = torch.where(is_ref, ref_frame_idx, gen_frame_idx) | ||
| ref_spatial_idx = position_idx % local_per_frame | ||
| gen_spatial_idx = gen_idx % local_per_frame | ||
| spatial_idx = torch.where(is_ref, ref_spatial_idx, gen_spatial_idx) |
There was a problem hiding this comment.
If local_per_frame is 0 (which can happen if num_new < frames or frames is 0), a ZeroDivisionError will be raised when computing ref_frames, ref_frame_idx, gen_frame_idx, ref_spatial_idx, and gen_spatial_idx. Guarding these computations with a check for local_per_frame > 0 prevents this runtime crash.
| ref_frames = ref_tokens // local_per_frame | |
| ref_frame_idx = position_idx // local_per_frame | |
| gen_frame_idx = ref_frames + gen_idx // local_per_frame | |
| frame_idx = torch.where(is_ref, ref_frame_idx, gen_frame_idx) | |
| ref_spatial_idx = position_idx % local_per_frame | |
| gen_spatial_idx = gen_idx % local_per_frame | |
| spatial_idx = torch.where(is_ref, ref_spatial_idx, gen_spatial_idx) | |
| if local_per_frame > 0: | |
| ref_frames = ref_tokens // local_per_frame | |
| ref_frame_idx = position_idx // local_per_frame | |
| gen_frame_idx = ref_frames + gen_idx // local_per_frame | |
| frame_idx = torch.where(is_ref, ref_frame_idx, gen_frame_idx) | |
| ref_spatial_idx = position_idx % local_per_frame | |
| gen_spatial_idx = gen_idx % local_per_frame | |
| spatial_idx = torch.where(is_ref, ref_spatial_idx, gen_spatial_idx) | |
| else: | |
| frame_idx = torch.zeros_like(position_idx) | |
| spatial_idx = torch.zeros_like(position_idx) |
| ref_frames: tl.constexpr = ref_tokens // local_per_frame | ||
| ref_frame_idx = token_idx // local_per_frame | ||
| gen_frame_idx = ref_frames + gen_idx // local_per_frame | ||
| frame_idx = tl.where(is_ref, ref_frame_idx, gen_frame_idx) | ||
| ref_spatial_idx = token_idx % local_per_frame | ||
| gen_spatial_idx = gen_idx % local_per_frame | ||
| local_spatial_idx = tl.where(is_ref, ref_spatial_idx, gen_spatial_idx) |
There was a problem hiding this comment.
If local_per_frame is 0, Triton compilation will fail with a compile-time division by zero error because ref_tokens and local_per_frame are both tl.constexpr. Since local_per_frame is a compile-time constant, we can use a Python conditional check if local_per_frame > 0: to safely guard these calculations and prevent compilation failures.
| ref_frames: tl.constexpr = ref_tokens // local_per_frame | |
| ref_frame_idx = token_idx // local_per_frame | |
| gen_frame_idx = ref_frames + gen_idx // local_per_frame | |
| frame_idx = tl.where(is_ref, ref_frame_idx, gen_frame_idx) | |
| ref_spatial_idx = token_idx % local_per_frame | |
| gen_spatial_idx = gen_idx % local_per_frame | |
| local_spatial_idx = tl.where(is_ref, ref_spatial_idx, gen_spatial_idx) | |
| if local_per_frame > 0: | |
| ref_frames: tl.constexpr = ref_tokens // local_per_frame | |
| ref_frame_idx = token_idx // local_per_frame | |
| gen_frame_idx = ref_frames + gen_idx // local_per_frame | |
| frame_idx = tl.where(is_ref, ref_frame_idx, gen_frame_idx) | |
| ref_spatial_idx = token_idx % local_per_frame | |
| gen_spatial_idx = gen_idx % local_per_frame | |
| local_spatial_idx = tl.where(is_ref, ref_spatial_idx, gen_spatial_idx) | |
| else: | |
| frame_idx = token_idx * 0 | |
| local_spatial_idx = token_idx * 0 |
| token_idx = token_start + token_head_idx | ||
| local_token_idx = token_start + token_head_idx | ||
| range_end = token_start + seq_len | ||
| recent_local_tokens = tl.maximum(range_end - sink_tokens, 0) |
There was a problem hiding this comment.
Using tl.maximum on purely compile-time scalar values (range_end and sink_tokens are both scalars/constexprs) can sometimes cause Triton compilation issues or unexpected scalar promotion behavior depending on the Triton version. Using a standard Python conditional expression is safer and guaranteed to be evaluated at compile time.
| recent_local_tokens = tl.maximum(range_end - sink_tokens, 0) | |
| recent_local_tokens = (range_end - sink_tokens) if (range_end - sink_tokens) > 0 else 0 |
| "use_31_block": true, | ||
| "dit_quantized": true, | ||
| "dit_quantized_ckpt": "/SekoTalk-Distill-AR/converted_fp8.safetensors", | ||
| "dit_quantized_ckpt": "/data/nvme5/gushiqiao/models/SekoTalk-Distill-AR-0703/seko_ar_new_rope_fp8_4steps.safetensors", |
There was a problem hiding this comment.
Hardcoding absolute paths with specific user directories (e.g., /data/nvme5/gushiqiao/...) makes the configuration non-portable and prone to breaking across different environments or users. Consider using relative paths, environment variables, or placeholder values.
| "dit_quantized_ckpt": "/data/nvme5/gushiqiao/models/SekoTalk-Distill-AR-0703/seko_ar_new_rope_fp8_4steps.safetensors", | |
| "dit_quantized_ckpt": "models/SekoTalk-Distill-AR-0703/seko_ar_new_rope_fp8_4steps.safetensors", |
| lightx2v_path=/data/nvme5/gushiqiao/codes/LightX2V | ||
| model_path=/data/nvme5/gushiqiao/models/SekoTalk-Distill-AR-0703/ |
There was a problem hiding this comment.
Hardcoding absolute paths with specific user directories (e.g., /data/nvme5/gushiqiao/...) makes the script non-portable. Consider using relative paths or environment variables to locate the repository and model directories.
| lightx2v_path=/data/nvme5/gushiqiao/codes/LightX2V | |
| model_path=/data/nvme5/gushiqiao/models/SekoTalk-Distill-AR-0703/ | |
| lightx2v_path=$(pwd) | |
| model_path=models/SekoTalk-Distill-AR-0703/ |
2 participants
No description provided.