diff --git a/.agents/handoffs/issue-1578-handoff.md b/.agents/handoffs/issue-1578-handoff.md deleted file mode 100644 index cdda487de..000000000 --- a/.agents/handoffs/issue-1578-handoff.md +++ /dev/null @@ -1,257 +0,0 @@ -# Issue 1578 Handoff - -## Metadata -- Issue: #1578 -- Title: `[Bug] Qwen2.5-VL vision tower always resolves to float32: torch_dtype read from vision_config, where it is never set` -- Repo: `hao-ai-lab/FastVideo` -- Branch: `issue/1578-qwen-vl-vision-dtype` -- Worktree: `/tmp/fastvideo-worktrees/issue-1578-qwen-vl-vision-dtype` -- Handoff path: `.agents/handoffs/issue-1578-handoff.md` -- Current stage: Stage 3 complete - awaiting explicit Stage 4 PR creation request -- Implementation begun: yes -- Created: 2026-07-10T08:36:53Z -- Last updated: 2026-07-10T10:56:00Z - -## Workflow State -- Stage 0 completed: no existing local or fetched `origin`/`upstream` branch containing `1578` was found. -- No existing `.agents/handoffs/issue-1578-handoff.md` was found in the main checkout or known FastVideo worktrees. -- Created dedicated worktree from `origin/main`. -- Verified `gh` identity as `macthecadillac`. -- `git fetch origin` completed with a known_hosts warning: `hostfile_replace_entries failed ... Invalid cross-device link`, exit code 0. -- `git fetch upstream` completed successfully. -- After the issue branch was created from stale `origin/main` (`9d909f5f0457ac91f489d5fc8000931f042b72ce`), it was fast-forwarded to current `upstream/main` (`4c08ffce497bb7fe732454135d1fb7bf6733fbb2`) with `git merge --ff-only upstream/main`. -- Branch status after fast-forward: ahead of `origin/main` by 15 commits, plus staged/unstaged handoff updates only. - -## Stage 1 Notes -- Stage 1 is implementation-free. Only this handoff has been created/updated. -- Root `AGENTS.md` read in the issue worktree. -- `fastvideo/AGENTS.md`, `fastvideo/models/AGENTS.md`, and `fastvideo/tests/AGENTS.md` read. -- Relevant lessons read: - - `.agents/lessons/2026-05-07_conversion-cast-bf16-suffix-allowlist.md` - - `.agents/lessons/2026-05-07_dit-dtype-boundary-with-flash-attn.md` -- Local config-only Python reproduction was attempted, but local environment lacks `torch`: - `ModuleNotFoundError: No module named 'torch'`. Do not rely on local tests; validate on Modal in Stage 2. - -## Stage 2 Notes -- User approved the recommended plan: "The plan looks good. Go ahead". -- Re-checked GitHub before editing: - - `gh api user --jq .login` returned `macthecadillac`. - - `gh issue view 1578 ...` still shows issue OPEN, no comments, no assignees. - - `gh pr list ... --state open` reviewed again. New PR #1579 is an unrelated LingBot model port. Related #1577 remains open and ready-for-review, covering #1576 test coverage only. - - Targeted `gh search prs --repo hao-ai-lab/FastVideo "Qwen2.5-VL"` still finds #1577 as the only open Qwen2.5-VL PR. - - Targeted `gh search prs --repo hao-ai-lab/FastVideo "vision_config torch_dtype"` still returns none. -- Selected approach: Approach A, local dtype normalization helper plus parent dtype propagation into the visual constructor. -- Patch applied: - - Added `_resolve_torch_dtype()` in `fastvideo/models/encoders/qwen2_5_vl_custom.py`. - - Changed `Qwen2_5_VisionTransformerPretrainedModel.__init__` to accept `parent_torch_dtype`, prefer explicit vision dtype, and fall back to parent dtype before defaulting to fp32. - - Changed `Qwen2_5_VLForConditionalGenerationSimple.__init__` to pass parent `config.torch_dtype` into the visual tower. - - Added `fastvideo/tests/encoders/test_qwen2_5_vl_vision_dtype.py` with tiny-config tests for parent string dtype, parent `torch.dtype`, vision override, fp32 fallback, and full parent-to-visual constructor propagation. -- Local syntax-only AST parse passed for: - - `fastvideo/models/encoders/qwen2_5_vl_custom.py` - - `fastvideo/tests/encoders/test_qwen2_5_vl_vision_dtype.py` -- Local pytest was not run. -- Modal L40S targeted validation passed with local patch applied: - - App: `ap-pZmxjLGQ8YtuIuz7c0TSta` - - Command: `pytest fastvideo/tests/encoders/test_qwen2_5_vl_vision_dtype.py -q` - - Result: `5 passed, 16 warnings in 0.23s` - - Launcher used `uvx --from modal modal run fastvideo/tests/modal/launch_l40s_job.py --install-extra none --apply-local-patch --command "pytest fastvideo/tests/encoders/test_qwen2_5_vl_vision_dtype.py -q"`. - - The first launch attempt with `python -m modal ...` failed locally before contacting Modal because `/home/toolbox/venv/bin/python` had no `modal` module. No repo state changed from that failure. -- Signed Stage 2 code commit: - - Commit: `67e394cb2e1a09831d78bb0ecd16edcf7b93f44d` - - Subject: `[bugfix]: propagate qwen vl visual dtype` - - Pushed to `origin/issue/1578-qwen-vl-vision-dtype`. -- Modal L40S targeted validation passed against pushed commit head: - - App: `ap-7bddOhh3JwhxxSE4nwL7tP` - - Commit: `67e394cb2e1a09831d78bb0ecd16edcf7b93f44d` - - Command: `pytest fastvideo/tests/encoders/test_qwen2_5_vl_vision_dtype.py -q` - - Result: `5 passed, 16 warnings in 0.19s` - - `local_patch_applied`: false. - -## Stage 3 Notes -- Started review/adjudication loop after pushed Stage 2 code. -- Read `/home/toolbox/.codex/skills/review-code/SKILL.md` before spawning reviewer. -- Spawned fresh review-code sub-agent: - - Agent id: `019f4b8e-6673-7871-a79a-ca33f41a7f44` - - Nickname: `Euclid` - - Prompt summary: review `macthecadillac/FastVideo` branch `issue/1578-qwen-vl-vision-dtype` for `hao-ai-lab/FastVideo#1578`, use `$review-code`, read-only, no GitHub connector, no code/GitHub mutations, no local tests. - - Status: completed. - - Findings: no actionable code findings. - - Issue fit: reviewer reported the branch addresses `hao-ai-lab/FastVideo#1578`. - - Related branches: only `issue/1578-qwen-vl-vision-dtype`; no open PR targets this branch or issue. - - Validation gaps: reviewer did not rerun tests, noted the existing Modal targeted pytest evidence. -- Because reviewer reported no actionable findings, no adjudicator/fixer sub-agent was spawned in this loop. -- Stage 3 pre-commit gate passed on Modal: - - App: `ap-PUuLQNwzBFA3D3Wz43815o` - - Commit checked out: `dee590b8826e38ea41aa8738b5e28b4dcab9af13` - - Local handoff patch applied: true - - Command: `pre-commit run --all-files` - - Result: passed (`yapf`, `ruff`, `codespell`, `PyMarkdown`, `actionlint`, `mypy`, filename spaces, suggestion all passed). - - Launcher used `uvx --from modal modal run fastvideo/tests/modal/launch_l40s_job.py --install-extra none --apply-local-patch --command "pre-commit run --all-files"`. -- No PR or draft PR has been opened. -- Handoff remains active and must be removed before Stage 4 PR creation. - -## Draft PR Message -Title: -`[bugfix]: propagate Qwen2.5-VL visual dtype` - -Body: -```markdown -## Summary - -Fixes #1578 by resolving the Qwen2.5-VL visual tower dtype from the parent config when `vision_config.torch_dtype` is absent. - -This change: -- normalizes `torch_dtype` values supplied as strings or `torch.dtype` objects -- keeps explicit `vision_config.torch_dtype` as the highest-priority source -- falls back to parent `config.torch_dtype` for real Qwen2.5-VL configs where dtype is top-level only -- preserves the existing fp32 fallback for missing or unknown dtype values -- adds CPU-only regression coverage for parent dtype propagation, dtype-object handling, vision override behavior, and fp32 fallback - -## Validation - -- Modal L40S targeted pytest against pushed branch commit `67e394cb2e1a09831d78bb0ecd16edcf7b93f44d`: - - `pytest fastvideo/tests/encoders/test_qwen2_5_vl_vision_dtype.py -q` - - `5 passed, 16 warnings in 0.19s` - - App: `ap-7bddOhh3JwhxxSE4nwL7tP` -- Earlier Modal L40S targeted pytest with local patch applied: - - `pytest fastvideo/tests/encoders/test_qwen2_5_vl_vision_dtype.py -q` - - `5 passed, 16 warnings in 0.23s` - - App: `ap-pZmxjLGQ8YtuIuz7c0TSta` -- Modal L40S pre-commit gate: - - `pre-commit run --all-files` - - passed - - App: `ap-PUuLQNwzBFA3D3Wz43815o` -- Stage 3 review loop: - - `review-code` sub-agent reported no actionable findings. - -## GPU Memory Impact - -No expected increase. For Qwen2.5-VL visual inputs, this avoids resolving `self.visual.dtype` to fp32 when the parent config declares bf16. The current text-only Reason1 path remains unchanged. - -## Notes - -- Wan T2V SSIM was not run because this change is limited to Qwen2.5-VL encoder dtype plumbing and does not affect Wan generated pixels. -- The support matrix was not updated because no new model or user-facing support claim was added. - -# Checklist -- [x] I ran pre-commit run --all-files and fixed all issues -- [x] I added or updated tests for my changes -- [x] I updated documentation if needed -- [x] I considered GPU memory impact of my changes -For model/pipeline changes, also check: -- [ ] I verified targeted Wan T2V SSIM regression tests pass on L40S -- [ ] I updated the support matrix if adding a new model -``` - -## GitHub Context -- `gh issue view 1578 -R hao-ai-lab/FastVideo --json ...` reviewed at 2026-07-10T08:40Z. -- Issue state: OPEN. -- URL: https://github.com/hao-ai-lab/FastVideo/issues/1578 -- Author: `Mister-Raggs`. -- Assignees: none. -- Labels: `installation`, `platform`, `scope: inference`, `scope: attention`, `scope: model`. -- Created: 2026-07-10T03:27:36Z. -- Updated: 2026-07-10T03:27:43Z. -- Comments: none. -- Reported bug: - - `fastvideo/models/encoders/qwen2_5_vl_custom.py` vision tower derives `self.dtype` from `config.torch_dtype`. - - The config passed to the vision tower is `config.vision_config`. - - Real `Qwen/Qwen2.5-VL-7B-Instruct` config stores `"torch_dtype": "bfloat16"` at the composite/top level, not in `vision_config`. - - Therefore `config.vision_config.torch_dtype` is `None`, and `self.dtype` resolves to `torch.float32`. - - `pixel_values` and `pixel_values_videos` are later cast to `self.visual.dtype`, so the image/video vision path casts inputs to fp32 while weights may be bf16. - - Reporter also notes a latent bug: comparing only to string `"bfloat16"` misses transformers versions that materialize `torch_dtype` as `torch.bfloat16`. - - Reporter believes image/video path is dormant in current Cosmos 2.5 Reason1 pipeline and has not run real-weight reproduction. -- Commenter-proposed fixes: none beyond reporter's suggestions: - - read dtype from parent/composite config; - - compare against both string and `torch.dtype`; - - or derive from actual module parameter dtype. -- Open PR sweep: - - `gh pr list -R hao-ai-lab/FastVideo --state open --limit 200 --json ...` reviewed. Large output included many unrelated open PRs. - - Targeted `gh search prs --repo hao-ai-lab/FastVideo "Qwen2.5-VL"` found #1577 as the only open Qwen2.5-VL PR. - - `gh search prs --repo hao-ai-lab/FastVideo "qwen2_5_vl_custom"` returned none. - - `gh search prs --repo hao-ai-lab/FastVideo "vision_config torch_dtype"` returned none. - - `gh search issues --repo hao-ai-lab/FastVideo "qwen2_5_vl_custom torch_dtype"` returned none. -- Related PR #1577: - - URL: https://github.com/hao-ai-lab/FastVideo/pull/1577 - - Title: `[ci]: cover transformers 5.x config compat for Qwen2.5-VL (#1576)` - - State: OPEN. - - Draft status: ready-for-review (`isDraft=false`); draft status was not changed. - - Branch: `test/qwen25vl-transformers5-compat`. - - Files: only `fastvideo/tests/encoders/test_qwen2_5_vl_config_compat.py` added. - - Covers transformers 5.x config API compatibility for #1576, not the production vision dtype resolution path in #1578. - -## Code Findings -- Current upstream code still has the reported bug. -- `fastvideo/models/encoders/qwen2_5_vl_custom.py:307-310`: - `Qwen2_5_VisionTransformerPretrainedModel.__init__` accepts only the vision sub-config and sets - `self.dtype = torch.bfloat16 if config.torch_dtype == "bfloat16" else torch.float32`. -- `fastvideo/models/encoders/qwen2_5_vl_custom.py:1457-1461`: - `Qwen2_5_VLForConditionalGenerationSimple.__init__` flattens text config fields, stores the parent config, then constructs - `self.visual = Qwen2_5_VisionTransformerPretrainedModel(config.vision_config)`. - It does not pass parent/composite `config.torch_dtype` to the visual tower. -- `_flatten_text_config()` only copies missing attributes from `text_config` to the parent config. It does not touch `vision_config` or dtype propagation. -- `fastvideo/models/encoders/qwen2_5_vl_custom.py:1744-1764`: - image and video inputs are explicitly cast with `pixel_values.type(self.visual.dtype)` and - `pixel_values_videos.type(self.visual.dtype)`. -- `fastvideo/models/encoders/qwen2_5_vl_custom.py:115-116`: - `Qwen2_5_VisionPatchEmbed.forward` then casts patch input to `self.proj.weight.dtype`. - This means the current code is unlikely to fail at the patch-embed conv solely because `self.visual.dtype` is fp32; it can still create an unnecessary fp32 temporary before casting back to the parameter dtype. -- `fastvideo/models/encoders/reason1.py:141-164`: - Reason1's loader currently skips `visual` weights, `lm_head`, and `decoder`, supporting the reporter's note that the image/video vision path is dormant for the current Cosmos 2.5 Reason1 usage. -- Searches run: - - `rg -n "Qwen2_5|qwen2_5|vision_config|torch_dtype|self\\.dtype|visual\\.dtype|pixel_values" fastvideo tests docs examples scripts` - - `rg -n "flatten_text_config|torch_dtype|Qwen2_5_VisionTransformerPretrainedModel|self\\.visual|visual\\.dtype" fastvideo/models/encoders/qwen2_5_vl_custom.py fastvideo/models/encoders/reason1.py fastvideo/tests/encoders` - - `rg -n "\\.type\\(self\\.visual\\.dtype\\)|visual\\.dtype|self\\.dtype =|target_dtype = .*weight\\.dtype" fastvideo/models/encoders/qwen2_5_vl_custom.py fastvideo/models/encoders` - -## Current Hypothesis -- The issue is valid: top-level `torch_dtype` is not visible to the vision-only sub-config, and the current string-only comparison also misses `torch.dtype` values. -- Severity is limited by current usage: - - Reason1 appears to use the text path and skip visual weights. - - Patch embed casts to parameter dtype internally, so the most likely practical effect is avoidable fp32 materialization and a misleading `self.visual.dtype`, not a guaranteed runtime conv dtype mismatch. -- The fix should be narrow and local to the custom Qwen2.5-VL model, with lightweight CPU/config tests. - -## Alternatives Considered -- Approach A - preferred: add a small dtype normalization helper and pass parent dtype into the visual constructor. - - Change `Qwen2_5_VisionTransformerPretrainedModel.__init__` to accept an optional parent/composite dtype. - - Prefer explicit `vision_config.torch_dtype` when present, otherwise fall back to parent `config.torch_dtype`. - - Normalize both strings such as `"bfloat16"` / `"torch.bfloat16"` and `torch.dtype` objects such as `torch.bfloat16`. - - Update `Qwen2_5_VLForConditionalGenerationSimple.__init__` to pass `getattr(config, "torch_dtype", None)` into the visual constructor. - - Tradeoff: small API change to an internal class, but no mutation of HF config objects and no runtime weight dependency. -- Approach B: mutate `config.vision_config.torch_dtype` from the parent config during `_flatten_text_config()`. - - Simpler constructor signature, but it mutates an external config object and mixes text-flattening compatibility with vision dtype propagation. - - Less preferred. -- Approach C: stop using `self.visual.dtype` and cast image/video tensors directly from the actual first visual parameter dtype. - - Most aligned with loaded weights, but this custom model can have meta/unloaded visual parameters in Reason1 and the public `self.visual.dtype` remains wrong unless also updated. - - More behavioral surface than needed for this report. -- Approach D: leave code unchanged because the active pipeline does not use the image/video path. - - Not preferred; the bug is real and easy to guard with low-cost tests. - -## Recommended Plan -- Use Approach A. -- Implementation steps for Stage 2 after user approval: - 1. Add a local helper near the Qwen2.5-VL visual classes, e.g. `_resolve_torch_dtype(dtype, default=torch.float32)`, that accepts `None`, `str`, and `torch.dtype`. - 2. Support at least `bfloat16`/`bf16`, `float16`/`fp16`/`half`, and `float32`/`fp32`/`float`, including optional `"torch."` prefixes. Unknown/`None` values should fall back to `torch.float32` to preserve current behavior. - 3. Change `Qwen2_5_VisionTransformerPretrainedModel.__init__(self, config, parent_torch_dtype=None)` so `vision_config.torch_dtype` wins when non-`None`, otherwise it uses the parent dtype. - 4. Change `Qwen2_5_VLForConditionalGenerationSimple.__init__` to pass parent `config.torch_dtype` when constructing `self.visual`. - 5. Add focused tests under `fastvideo/tests/encoders/`, likely `test_qwen2_5_vl_vision_dtype.py`, using tiny `types.SimpleNamespace` configs so no HF download or real weights are required. - 6. Cover parent string `"bfloat16"`, parent `torch.bfloat16`, explicit vision override, and fallback/default fp32 behavior. - 7. Keep the fix scoped to dtype resolution; do not refactor Reason1 loading or enable the dormant visual path. -- Compatibility concerns: - - Preserves current fp32 fallback for absent or unknown dtype. - - Should not alter text-only Reason1 behavior. - - No expected GPU memory increase; expected to reduce avoidable fp32 image/video input temporary allocation when the visual path is used with bf16. -- Documentation impact: none expected; this is internal model dtype plumbing. - -## Validation Plan -- Do not run local tests; local environment is missing `torch` and FastVideo rules require Modal validation. -- Stage 2 targeted validation on Modal L40S from the issue worktree after implementation: - `python -m modal run fastvideo/tests/modal/launch_l40s_job.py --install-extra none --command "pytest fastvideo/tests/encoders/test_qwen2_5_vl_vision_dtype.py -q"` -- If the implementation touches only this helper/model path and CPU-only tests, full SSIM is not warranted; no generated-pixel path should change for existing text-only pipelines. -- Before any future draft PR creation, run mandatory pre-PR gate: - `pre-commit run --all-files` -- Future PR creation, if requested, must create only a draft PR if no PR exists, and must not change any existing PR draft status. - -## Next Steps -1. Stage and commit/push this handoff-only Stage 1 state with GPG signing. -2. Present Stage 1 report to the user. -3. Await user guidance before any implementation.