Skip to content

[https://nvbugs/6410093][fix] updated cosmos3 lpips baseline#16436

Open
NVShreyas wants to merge 4 commits into
NVIDIA:mainfrom
NVShreyas:user/shreyasm/cosmos3-new-lpips
Open

[https://nvbugs/6410093][fix] updated cosmos3 lpips baseline#16436
NVShreyas wants to merge 4 commits into
NVIDIA:mainfrom
NVShreyas:user/shreyasm/cosmos3-new-lpips

Conversation

@NVShreyas

@NVShreyas NVShreyas commented Jul 15, 2026

Copy link
Copy Markdown
Collaborator

This PR upgrades LPIPS baseline for cosmos3, fixing nvbugs - 6410093, 6418815, 6437341.

In a previous PR, cosmos3 transformer was updated to not pad inputs when running on 1 GPU, instead running attention forward for each of the 2 CFG batches. This fixed a bug causing accuracy degradation.
The current outputs were generated by running the LPIPS script manually and updating the golden references. The outputs have been visually validated and should be correct going forward.

Summary by CodeRabbit

  • Tests
    • Removed two obsolete test waivers for Cosmos3 visual generation comparisons.
    • These comparisons will now run as part of the standard test suite.

Description

Test Coverage

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.

@NVShreyas

Copy link
Copy Markdown
Collaborator Author

/bot help

@github-actions

Copy link
Copy Markdown

GitHub Bot Help

/bot [-h] ['run', 'kill', 'skip', 'reuse-pipeline'] ...

Provide a user friendly way for developers to interact with a Jenkins server.

Run /bot [-h|--help] to print this help message.

See details below for each supported subcommand.

Details

run [--reuse-test (optional)pipeline-id --disable-fail-fast --skip-test --stage-list "A10-PyTorch-1, xxx" --gpu-type "A30, H100_PCIe" --test-backend "pytorch, cpp" --add-multi-gpu-test --only-multi-gpu-test --disable-multi-gpu-test --post-merge --extra-stage "H100_PCIe-TensorRT-Post-Merge-1, xxx" --detailed-log --debug(experimental) --high-priority]

Launch build/test pipelines. All previously running jobs will be killed.

--reuse-test (optional)pipeline-id (OPTIONAL) : Allow the new pipeline to reuse build artifacts and skip successful test stages from a specified pipeline or the last pipeline if no pipeline-id is indicated. If the Git commit ID has changed, this option will be always ignored. The DEFAULT behavior of the bot is to reuse build artifacts and successful test results from the last pipeline.

--disable-reuse-test (OPTIONAL) : Explicitly prevent the pipeline from reusing build artifacts and skipping successful test stages from a previous pipeline. Ensure that all builds and tests are run regardless of previous successes.

--disable-fail-fast (OPTIONAL) : Disable fail fast on build/tests/infra failures.

--skip-test (OPTIONAL) : Skip all test stages, but still run build stages, package stages and sanity check stages. Note: Does NOT update GitHub check status.

--stage-list "A10-PyTorch-1, xxx" (OPTIONAL) : Only run the specified test stages. Supports wildcard * for pattern matching (e.g., "*PerfSanity*" matches all stages containing PerfSanity). Examples: "A10-PyTorch-1, xxx", "PerfSanity". Note: Does NOT update GitHub check status.

--gpu-type "A30, H100_PCIe" (OPTIONAL) : Only run the test stages on the specified GPU types. Examples: "A30, H100_PCIe". Note: Does NOT update GitHub check status.

--test-backend "pytorch, cpp" (OPTIONAL) : Skip test stages which don't match the specified backends. Only support [pytorch, cpp, tensorrt, triton]. Examples: "pytorch, cpp" (does not run test stages with tensorrt or triton backend). Note: Does NOT update GitHub pipeline status.

--only-multi-gpu-test (OPTIONAL) : Only run the multi-GPU tests. Note: Does NOT update GitHub check status.

--disable-multi-gpu-test (OPTIONAL) : Disable the multi-GPU tests. Note: Does NOT update GitHub check status.

--add-multi-gpu-test (OPTIONAL) : Force run the multi-GPU tests in addition to running L0 pre-merge pipeline.

--post-merge (OPTIONAL) : Run the L0 post-merge pipeline instead of the ordinary L0 pre-merge pipeline.

--extra-stage "H100_PCIe-TensorRT-Post-Merge-1, xxx" (OPTIONAL) : Run the ordinary L0 pre-merge pipeline and specified test stages. Supports wildcard * for pattern matching. Examples: --extra-stage "H100_PCIe-TensorRT-Post-Merge-1, xxx", --extra-stage "Post-Merge".

--detailed-log (OPTIONAL) : Enable flushing out all logs to the Jenkins console. This will significantly increase the log volume and may slow down the job.

--debug (OPTIONAL) : Experimental feature. Enable access to the CI container for debugging purpose. Note: Specify exactly one stage in the stage-list parameter to access the appropriate container environment. Note: Does NOT update GitHub check status.

--high-priority (OPTIONAL) : Run the pipeline with high priority. This option is restricted to authorized users only and will route the job to a high-priority queue.

kill

kill

Kill all running builds associated with pull request.

skip

skip --comment COMMENT

Skip testing for latest commit on pull request. --comment "Reason for skipping build/test" is required. IMPORTANT NOTE: This is dangerous since lack of user care and validation can cause top of tree to break.

reuse-pipeline

reuse-pipeline

Reuse a previous pipeline to validate current commit. This action will also kill all currently running builds associated with the pull request. IMPORTANT NOTE: This is dangerous since lack of user care and validation can cause top of tree to break.

@NVShreyas NVShreyas changed the title [https://nvbugspro/6410093][fix] updated cosmos3 lpips baseline [https://nvbugs/6410093][fix] updated cosmos3 lpips baseline Jul 15, 2026
@NVShreyas

Copy link
Copy Markdown
Collaborator Author

/bot run --post-merge --disable-multi-gpu-test

@tensorrt-cicd

Copy link
Copy Markdown
Collaborator

PR_Github #59468 [ run ] triggered by Bot. Commit: f70aab9 Link to invocation

@tensorrt-cicd

Copy link
Copy Markdown
Collaborator

PR_Github #59468 [ run ] completed with state SUCCESS. Commit: f70aab9
/LLM/main/L0_MergeRequest_PR pipeline #47935 (Partly Tested) completed with status: 'FAILURE'

CI Report

⚠️ Action Required:

  • Please check the failed tests and fix your PR
  • If you cannot view the failures, ask the CI triggerer to share details
  • Once fixed, request an NVIDIA team member to trigger CI again

CI Agent Failure Analysis

Link to invocation

@NVShreyas

Copy link
Copy Markdown
Collaborator Author

/bot run --post-merge --disable-multi-gpu-test

@NVShreyas

Copy link
Copy Markdown
Collaborator Author

/bot kill

@coderabbitai

coderabbitai Bot commented Jul 15, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Walkthrough

Removes two skip entries so Cosmos3 visual generation LPIPS comparisons for nano_t2i and nano_t2v run against golden outputs.

Changes

Visual generation validation

Layer / File(s) Summary
Remove Cosmos3 LPIPS waivers
tests/integration/test_lists/waives.txt
Removes the waivers for the Cosmos3 nano_t2i and nano_t2v golden-output comparisons.

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

Possibly related PRs

Suggested reviewers: schetlur-nv, brnguyen2, yuanjingx87, yiqingy0, fredricz-20070104

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
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.
Title check ✅ Passed The title matches the PR scope and follows the required ticket-type-summary format.
Description check ✅ Passed The description explains the baseline update and rationale, but it omits explicit test coverage details.
✨ 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 #59501 [ run ] triggered by Bot. Commit: fe32298 Link to invocation

@tensorrt-cicd

Copy link
Copy Markdown
Collaborator

PR_Github #59502 [ kill ] triggered by Bot. Commit: fe32298 Link to invocation

@tensorrt-cicd

Copy link
Copy Markdown
Collaborator

PR_Github #59501 [ run ] completed with state ABORTED. Commit: fe32298

Link to invocation

@tensorrt-cicd

Copy link
Copy Markdown
Collaborator

PR_Github #59502 [ kill ] completed with state SUCCESS. Commit: fe32298
Successfully killed previous jobs for commit fe32298

Link to invocation

@NVShreyas

Copy link
Copy Markdown
Collaborator Author

/bot kill

@NVShreyas

Copy link
Copy Markdown
Collaborator Author

/bot run --post-merge --disable-multi-gpu-test

@tensorrt-cicd

Copy link
Copy Markdown
Collaborator

PR_Github #59732 [ run ] triggered by Bot. Commit: 0e0d951 Link to invocation

@tensorrt-cicd

Copy link
Copy Markdown
Collaborator

PR_Github #59732 [ run ] completed with state SUCCESS. Commit: 0e0d951
/LLM/main/L0_MergeRequest_PR pipeline #48159 (Partly Tested) completed with status: 'FAILURE'

CI Report

⚠️ Action Required:

  • Please check the failed tests and fix your PR
  • If you cannot view the failures, ask the CI triggerer to share details
  • Once fixed, request an NVIDIA team member to trigger CI again

CI Agent Failure Analysis

Link to invocation

@fredricz-20070104

Copy link
Copy Markdown
Collaborator

Thanks for the fix — the approach (refresh the LPIPS golden baseline + drop the waivers to re-enable the cases) is the right way to handle LPIPS drift. Two small asks before this is merged:

  1. Please document how the new golden baseline was generated/validated. Swapping the golden to the current model output makes the test pass trivially, so it would help to have a note (in the PR description or here) confirming the new baseline output was human-verified as the correct expected output rather than freezing a real quality regression into the baseline. A short description of how visual_gen_lpips_golden_media.zip was regenerated and checked would be enough.

  2. Please list the bugs actually fixed here in the title/description. The title references nvbugs/6410093, but on main that bug has no waiver — this PR actually removes the waivers for nvbugs/6418815 (t2i) and nvbugs/6437341 (t2v). Since 6410093 and 6437341 are the same t2v failure (duplicates), re-enabling t2v with the new baseline does effectively cover 6410093 too, but it would avoid confusion to also list 6418815 and 6437341 explicitly.

Happy to approve once these are addressed.

@fredricz-20070104 fredricz-20070104 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Please address the above comments before merging.

@NVShreyas

Copy link
Copy Markdown
Collaborator Author

Thanks for the fix — the approach (refresh the LPIPS golden baseline + drop the waivers to re-enable the cases) is the right way to handle LPIPS drift. Two small asks before this is merged:

  1. Please document how the new golden baseline was generated/validated. Swapping the golden to the current model output makes the test pass trivially, so it would help to have a note (in the PR description or here) confirming the new baseline output was human-verified as the correct expected output rather than freezing a real quality regression into the baseline. A short description of how visual_gen_lpips_golden_media.zip was regenerated and checked would be enough.
  2. Please list the bugs actually fixed here in the title/description. The title references nvbugs/6410093, but on main that bug has no waiver — this PR actually removes the waivers for nvbugs/6418815 (t2i) and nvbugs/6437341 (t2v). Since 6410093 and 6437341 are the same t2v failure (duplicates), re-enabling t2v with the new baseline does effectively cover 6410093 too, but it would avoid confusion to also list 6418815 and 6437341 explicitly.

Happy to approve once these are addressed.

Thanks for your comments @fredricz-20070104. I have addressed both in the PR description.

@NVShreyas

Copy link
Copy Markdown
Collaborator Author

/bot run --post-merge --disable-multi-gpu-test --disable-fail-fast

@tensorrt-cicd

Copy link
Copy Markdown
Collaborator

PR_Github #60005 [ run ] triggered by Bot. Commit: 0e0d951 Link to invocation

@tensorrt-cicd

Copy link
Copy Markdown
Collaborator

PR_Github #60005 [ run ] completed with state FAILURE. Commit: 0e0d951
/LLM/main/L0_MergeRequest_PR pipeline #48396 (Partly Tested) completed with status: 'FAILURE'

CI Report

⚠️ Action Required:

  • Please check the failed tests and fix your PR
  • If you cannot view the failures, ask the CI triggerer to share details
  • Once fixed, request an NVIDIA team member to trigger CI again

CI Agent Failure Analysis

Link to invocation

Signed-off-by: Shreyas Misra <shreyasm@nvidia.com>
Signed-off-by: Shreyas Misra <shreyasm@nvidia.com>
@NVShreyas
NVShreyas force-pushed the user/shreyasm/cosmos3-new-lpips branch from 0e0d951 to a7d7804 Compare July 21, 2026 00:24
@NVShreyas

Copy link
Copy Markdown
Collaborator Author

/bot run --post-merge --disable-multi-gpu-test --disable-fail-fast

@tensorrt-cicd

Copy link
Copy Markdown
Collaborator

PR_Github #60506 [ run ] triggered by Bot. Commit: a7d7804 Link to invocation

@tensorrt-cicd

Copy link
Copy Markdown
Collaborator

PR_Github #60506 Bot args parsing error: Post-merge CI requested by --post-merge or a broad stage selector ('*' or 'Post-Merge') requires the ci: post-merge approved PR label. Ask a member of NVIDIA/trt-llm-ci-approvers to apply it.

Link to invocation

@NVShreyas NVShreyas added the ci: post-merge approved Approved by TRT-LLM CI approvers for broad post-merge CI requests label Jul 21, 2026
@github-actions

Copy link
Copy Markdown

Removed the "ci: post-merge approved" label because @NVShreyas could not be verified as an active member of NVIDIA/trt-llm-ci-approvers. Ask a member of that team to apply it.

@github-actions github-actions Bot removed the ci: post-merge approved Approved by TRT-LLM CI approvers for broad post-merge CI requests label Jul 21, 2026
@zhenhuaw-me
zhenhuaw-me requested a review from a team July 22, 2026 00:59
Signed-off-by: NVShreyas <158103197+NVShreyas@users.noreply.github.com>
@juney-nvidia juney-nvidia added ci: post-merge approved Approved by TRT-LLM CI approvers for broad post-merge CI requests and removed ci: post-merge approved Approved by TRT-LLM CI approvers for broad post-merge CI requests labels Jul 23, 2026
Comment thread tests/integration/test_lists/waives.txt Outdated
Co-authored-by: Zhenhua Wang <4936589+zhenhuaw-me@users.noreply.github.com>
Signed-off-by: NVShreyas <158103197+NVShreyas@users.noreply.github.com>
@NVShreyas

Copy link
Copy Markdown
Collaborator Author

/bot run --post-merge --disable-multi-gpu-test --disable-fail-fast

@tensorrt-cicd

Copy link
Copy Markdown
Collaborator

PR_Github #61195 [ run ] triggered by Bot. Commit: 343d98e Link to invocation

@tensorrt-cicd

Copy link
Copy Markdown
Collaborator

PR_Github #61195 Bot args parsing error: Post-merge CI requested by --post-merge or a broad stage selector ('*' or 'Post-Merge') requires the ci: post-merge approved PR label. Ask a member of NVIDIA/trt-llm-ci-approvers to apply it.

Link to invocation

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.

6 participants