Skip to content

[https://nvbugs/6550708][test] Unwaive Cosmos3 distilled unit tests fixed by #17212 - #17722

Merged
chang-l merged 1 commit into
NVIDIA:mainfrom
ishovkun:bugfix-negative-6550708
Aug 14, 2026
Merged

[https://nvbugs/6550708][test] Unwaive Cosmos3 distilled unit tests fixed by #17212#17722
chang-l merged 1 commit into
NVIDIA:mainfrom
ishovkun:bugfix-negative-6550708

Conversation

@ishovkun

@ishovkun ishovkun commented Aug 14, 2026

Copy link
Copy Markdown
Contributor

Dev Engineer Review

  • Removed four outdated Cosmos3 distilled test waivers from tests/integration/test_lists/waives.txt.
  • The change has no source, API, performance, or error-handling impact.
  • The waiver list change is correctly scoped to four removals.
  • The shared _profiler=VisualGenProfiler() fixture fix restores the affected tests.

QA Engineer Review

  • Modified files: none under test-db/ or qa/.
  • Removed four Cosmos3 distilled test entries from tests/integration/test_lists/waives.txt.
  • The tests cover distilled denoise-loop behavior, scheduler-step keyword propagation, and clean conditioning-frame preservation.
  • Verdict: needs follow-up. CBTS coverage data is unavailable.

Description

#16814 replaced the enumerate(timesteps) loop in BasePipeline.denoise() with
self._profile_denoise_steps(...), which made _profiler mandatory for the shared
denoise loop. The Cosmos3 unit-test double builds its pipeline through
object.__new__, so it never had that attribute, and four tests in
tests/unittest/_torch/visual_gen/test_cosmos3_distilled.py began failing with:

AttributeError: 'Cosmos3OmniMoTPipeline' object has no attribute '_profiler'

They were waived under https://nvbugs/6550708 on 2026-08-02.

#17212 fixed the fixture on 2026-08-03 by stubbing _profiler=VisualGenProfiler() in
_denoise_ready_pipeline() — the helper all four tests go through — but the four
waiver lines were never removed, so the tests have stayed off for roughly ten days
after their bug was fixed.

This PR deletes those four lines from tests/integration/test_lists/waives.txt.
No source changes, and no other waiver is touched.

Test Coverage

The four tests being re-enabled:

  • TestDistilledDenoiseLoop::test_guidance_one_single_forward_per_step
  • TestDistilledDenoiseLoop::test_scheduler_step_kwargs_reach_every_step
  • TestDistilledConditioningAnchor::test_every_forward_sees_clean_conditioned_frame
  • TestDistilledConditioningAnchor::test_without_anchor_the_conditioned_frame_drifts

They guard the distilled Cosmos3 denoise loop: exactly one forward per step at
guidance_scale=1.0, scheduler_step_kwargs (which carries the seeded generator for
per-step SDE noise) reaching every step, and the conditioning anchor keeping frame 0
clean across the loop. That path has no other working CI coverage today — its
end-to-end LPIPS gate does not execute because its checkpoint is missing from CI's
LLM_MODELS_ROOT (tracked separately).

unittest/_torch/visual_gen/test_cosmos3_distilled.py is listed in
tests/integration/test_lists/test-db/l0_b200.yml under stage: pre_merge, backend: pytorch, so this PR's own pipeline exercises the four tests; results will be
confirmed PASSED rather than SKIPPED before merge.

PR Checklist

Please review the following before submitting your PR:

  • PR description clearly explains what and why. If using CodeRabbit's summary, please make sure it makes sense.

  • PR Follows TRT-LLM CODING GUIDELINES to the best of your knowledge.

  • Test cases are provided for new code paths (see test instructions)

  • If PR introduces API changes, an appropriate PR label is added - either api-compatible or api-breaking. For api-breaking, include BREAKING in the PR title.

  • Any new dependencies have been scanned for license and vulnerabilities

  • CODEOWNERS updated if ownership changes

  • Documentation updated as needed

  • Update tava architecture diagram if there is a significant design change in PR.

  • The reviewers assigned automatically/manually are appropriate for the PR.

  • Please check this after reviewing the above items as appropriate for this PR.

GitHub Bot Help

To see a list of available CI bot commands, please comment /bot help.

Four tests in test_cosmos3_distilled.py were waived after NVIDIA#16814 made
BasePipeline.denoise() call self._profile_denoise_steps(), which the
Cosmos3 test double lacked because it builds its pipeline through
object.__new__ and so never got a _profiler attribute.

NVIDIA#17212 fixed that on 2026-08-03 by stubbing _profiler=VisualGenProfiler()
in _denoise_ready_pipeline(), the helper all four tests go through, but
the waivers were left behind. Remove them so the distilled denoise loop
is covered again.

No source changes.

Signed-off-by: Igor Shovkun <ishovkun@nvidia.com>
@ishovkun

Copy link
Copy Markdown
Contributor Author

/bot run --disable-fail-fast

@coderabbitai

coderabbitai Bot commented Aug 14, 2026

Copy link
Copy Markdown
Contributor

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: d723a6dd-c10b-4198-8e6a-00ef35d465de

📥 Commits

Reviewing files that changed from the base of the PR and between 4e95cb7 and d627837.

📒 Files selected for processing (1)
  • tests/integration/test_lists/waives.txt
💤 Files with no reviewable changes (1)
  • tests/integration/test_lists/waives.txt

Walkthrough

The change removes four skipped Cosmos3 distilled visual-generation tests from the integration test waivers list.

Changes

Cohort / File(s) Summary
Waiver list cleanup
tests/integration/test_lists/waives.txt
Removed four skipped Cosmos3 visual-generation test entries.

Estimated code review effort: 1 (Trivial) | ~2 minutes

Merge Risk: ⚪ Minimal · up to d6278

This change re-enables four previously fixed Cosmos3 unit tests without altering production code or runtime behavior; no actionable merge-blocking risk remains beyond normal checks.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly identifies the test-related change, the Cosmos3 tests, and the bug fix that enables their reactivation.
Description check ✅ Passed The description explains the cause, solution, affected tests, test coverage, and checklist status in sufficient detail.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Comment @coderabbitai help to get the list of available commands.

@tensorrt-cicd

Copy link
Copy Markdown
Collaborator

PR_Github #66382 [ run ] triggered by Bot. Commit: d627837 Link to invocation

@tensorrt-cicd

Copy link
Copy Markdown
Collaborator

PR_Github #66382 [ run ] completed with state SUCCESS. Commit: d627837
/LLM/main/L0_MergeRequest_PR pipeline #54025 completed with status: 'SUCCESS'

CI Report

Link to invocation

@chang-l
chang-l merged commit 91107ed into NVIDIA:main Aug 14, 2026
12 checks passed
xinhe-nv pushed a commit to xinhe-nv/TensorRT-LLM that referenced this pull request Aug 17, 2026
…ixed by NVIDIA#17212 (NVIDIA#17722)

Signed-off-by: Igor Shovkun <ishovkun@nvidia.com>
yihwang-nv pushed a commit to yihwang-nv/TensorRT-LLM that referenced this pull request Aug 18, 2026
…ixed by NVIDIA#17212 (NVIDIA#17722)

Signed-off-by: Igor Shovkun <ishovkun@nvidia.com>
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.

4 participants