Skip to content

test: pin Wan CausalConv3d streaming cache (PRPUNDIT-24) - #1086

Open
jiagaoxiang wants to merge 1 commit into
AMD-AGI:mainfrom
jiagaoxiang:testgap/PRPUNDIT-24
Open

test: pin Wan CausalConv3d streaming cache (PRPUNDIT-24)#1086
jiagaoxiang wants to merge 1 commit into
AMD-AGI:mainfrom
jiagaoxiang:testgap/PRPUNDIT-24

Conversation

@jiagaoxiang

@jiagaoxiang jiagaoxiang commented Sep 3, 2026

Copy link
Copy Markdown
Collaborator

This PR was created by dougljia via Test Gap Resolver.

Summary

  • One parameterized CPU suite covers both Wan 2.1 vae2_1.py::CausalConv3d.forward and Wan 2.2 vae2_2.py::CausalConv3d.forward.
  • Chunked streaming reuses the last CACHE_T frames of the previous chunk as cache_x and must match a single full-sequence pass; an independent F.pad + Conv3d oracle pins the cache-splice / reduced-front-padding arithmetic.
  • Follows linxwang's note that PRPUNDIT-24 consolidates both implementations into one parameterized streaming-cache test (do not file a second PR for vae2_2.py).

Closes test gap PRPUNDIT-24.

Test plan

  • PYTHONPATH=. pytest tests/unit_tests/backends/diffusion/test_wan_vae_causal_conv3d.py (12 passed, CPU: 6 tests × 2 VAE modules)
  • black --check on the new file

PRPUNDIT-24 covers both vae2_1 and vae2_2 in one parameterized suite:
chunked cache_x reuse versus a full-sequence pass, plus an independent
F.pad + Conv3d oracle.
Copilot AI lite review requested due to automatic review settings September 3, 2026 16:20

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Approval recommended

The change is isolated to a new test file and accurately exercises the existing CausalConv3d cache/padding behavior in both targeted modules.

Pull request overview

Adds a parameterized unit-test suite to pin CausalConv3d.forward streaming-cache behavior for both Wan 2.1 and Wan 2.2 VAE implementations, ensuring chunked streaming output matches a full-sequence pass and validating the cache-splice/front-padding arithmetic via an independent F.pad + F.conv3d oracle.

Changes:

  • Introduces a single parameterized test module covering both vae2_1.py and vae2_2.py CausalConv3d implementations.
  • Validates full-sequence output against an independent padding+conv oracle and asserts expected causal padding layout.
  • Verifies chunked streaming with cached frames (CACHE_T) matches full-sequence output, including full/partial/no-temporal-padding cache cases.
File summaries
File Description
tests/unit_tests/backends/diffusion/test_wan_vae_causal_conv3d.py New parameterized unit tests that pin causal temporal padding and streaming-cache equivalence across Wan 2.1 and 2.2 VAE modules.
Review details
  • Files reviewed: 1/1 changed files
  • Comments generated: 0
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@jiagaoxiang

Copy link
Copy Markdown
Collaborator Author

TestIntel PR Steward: Checked in on this PR after the poll flagged CI failure + a new review.

CI — build-docker (3.12) failure: not caused by this change. The Docker image itself built fine (all 14 layers, mostly cached, naming to docker.io/tasimage/primus:pr-1086 done). The job then fails at the registry-push step:

docker login -u tasimage -p
flag needs an argument: 'p' in -p
##[error]Process completed with exit code 125.

The -p password argument is empty, which reads as the Docker Hub credentials secret not being resolved for this run — a known GitHub Actions restriction on fork-originated pull requests (secrets aren't passed to pull_request-triggered workflows from forks by default). This PR only touches tests/unit_tests/backends/diffusion/test_wan_vae_causal_conv3d.py, so there's no code path here that affects docker build/push. Since the root cause is a missing/unavailable secret rather than a transient runner issue, rerunning is unlikely to change the outcome — this needs a maintainer to either approve the workflow run with secrets or adjust the docker-build job to skip the push step for fork PRs. Leaving CI as-is rather than reworking unrelated docker-build/CI config from this PR.

Copilot review: recommended approval, 0 inline comments — nothing actionable to address.

Still waiting on a human approval before this can merge; will keep watching.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants