Skip to content

[None][test] Add coverage for MBartForConditionalGeneration - #22171

Open
StanleySun639 wants to merge 1 commit into
NVIDIA:mainfrom
StanleySun639:gap-fixer/job-107-draft-151-check-123
Open

StanleySun639 wants to merge 1 commit into
NVIDIA:mainfrom
StanleySun639:gap-fixer/job-107-draft-151-check-123

Conversation

@StanleySun639

@StanleySun639 StanleySun639 commented Nov 12, 2026 •

Copy link
Copy Markdown
Collaborator

Dev Engineer Review

  • No source code changed. The only changed file is tests/unittest/_torch/modeling/test_modeling_bart.py (+86/-2).
  • The diff adds test_mbart_for_conditional_generation_construction_and_forward. The test is skipped when CUDA is unavailable.
  • The test imports MBartForConditionalGeneration from tensorrt_llm._torch.models.modeling_bart. The diff does not change that module.
  • The test checks registry resolution with get_registered_model_class("MBartForConditionalGeneration"). It checks layer-norm presence and normalize_before for both stacks. It checks cross-attention metadata and wrapper forward output against the Hugging Face reference.
  • Reference tolerances are rtol=2e-4 and atol=2e-5. The test sets _attn_implementation = "eager" on the reference model.
  • No API, default, or runtime behavior changes were found in the diff.

QA Engineer Review

  • Test file modified: tests/unittest/_torch/modeling/test_modeling_bart.py.
  • Test function added: test_mbart_for_conditional_generation_construction_and_forward.
  • Existing test test_bart_for_conditional_generation_construction_and_forward is unchanged in this diff.
  • Covered behavior:
    • Production registry resolution for MBartForConditionalGeneration.
    • Model construction and layer-norm and normalize_before settings for encoder and decoder.
    • Cross-attention metadata creation.
    • Wrapper forward output shape, dtype (float32), finiteness, and closeness to the Hugging Face mBART reference.
  • The existing file path is listed in tests/integration/test_lists/test-db/l0_a10.yml (line 30). The file is therefore run in CI through that entry. The new test function is covered by the file-level entry.
  • Coverage verdict: sufficient for the stated scope. The test requires CUDA and does not run on CPU-only runners.
  • CI status: the author reports Jenkins #107 as SUCCESS. This summary did not verify that result.

Per-File QA Perspective

  • tests/unittest/_torch/modeling/test_modeling_bart.py: Adds an mBART construction and forward check against the Hugging Face reference. It is listed in CI through l0_a10.yml. A QA engineer should confirm the CUDA-gated test runs on an A10 runner, not only skips.

Description

Adds unit-test coverage for existing supported model architectures.

  • tests/unittest/_torch/modeling/test_modeling_bart.py::test_mbart_for_conditional_generation_construction_and_forward verifies: The production registry resolves MBartForConditionalGeneration to the exact production class, and constructing that class produces an MBartForConditionalGeneration instance. Both model stacks have final layer normalization and their single layers use pre-normalization. The prepared stateless cross-attention metadata identifies cross-attention. The real wrapper forward returns finite float32 logits with shape (3, 64). Every decoder context token's logits match an independently executed Hugging Face mBART model with identical weights, within rtol=2e-4 and atol=2e-5.

Test Coverage

  • Test case: unittest/_torch/modeling/test_modeling_bart.py::test_mbart_for_conditional_generation_construction_and_forward
  • Jenkins log: Jenkins #107 — SUCCESS

Gap Fixer Validation

  • Patch review: Passed with no unresolved must-fix findings.
  • Analyzed source snapshot: 461580bcd5ad
  • Gap Fixer run: Job #107

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.

Signed-off-by: Stanley Sun <stsun@nvidia.com>
@StanleySun639
StanleySun639 marked this pull request as ready for review November 12, 2026 07:30
@StanleySun639
StanleySun639 requested a review from a team as a code owner November 12, 2026 07:30
@coderabbitai

coderabbitai Bot commented Nov 12, 2026

Copy link
Copy Markdown
Contributor

Explain Change →

No actionable comments were generated in the recent review.

Walkthrough

This change adds a CUDA-gated mBART test. It compares TensorRT-LLM encoder-decoder inference with a Hugging Face reference using matching configuration and weights.

Changes

mBART inference validation

Layer / File(s) Summary
Model construction and inference validation
tests/unittest/_torch/modeling/test_modeling_bart.py
The test imports mBART classes, checks model registration and normalization properties, and compares decoder logits with a Hugging Face reference for shape, dtype, finiteness, and numerical closeness.

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Other

Merge Risk: Minimal · up to cb634

This change adds test coverage for the mBART model and does not alter runtime behavior. No merge-blocking risk is evident.

Pre-merge checks: Passed 4

Check name Status Explanation
Title check Passed The title names the main change: adding test coverage for MBartForConditionalGeneration. It follows the [None][test] ticket-and-type format. The title is short and specific, and it matches the test fi…
Description check Passed The description covers the purpose, the test case, and the validation results. It includes a Description section, a Test Coverage section with the test name and Jenkins log, and a checked PR Checklist…
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.
Full details: Title check

Explanation

The title names the main change: adding test coverage for MBartForConditionalGeneration. It follows the [None][test] ticket-and-type format. The title is short and specific, and it matches the test file in the raw summary.

Full details: Description check

Explanation

The description covers the purpose, the test case, and the validation results. It includes a Description section, a Test Coverage section with the test name and Jenkins log, and a checked PR Checklist. It omits the template's ticket-and-type title format note only because the title already follows it. The Description section is a single sentence and does not explain the issue or solution in depth, but the test-level detail compensates for this. The description still keeps the template's HTML comment block at the top.

Finishing Touches

Generate unit tests (beta)

  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Recent review info

Run configuration

Commits

Reviewing files that changed from the base of the PR and between c046811 and cb6347c.

Files selected for processing: 1

  • tests/unittest/_torch/modeling/test_modeling_bart.py

Included review availability: This review used your included allowance. Your plan provides up to 12 included reviews per hour; 8 remain after this review.

@StanleySun639

Copy link
Copy Markdown
Collaborator Author

/bot run

@tensorrt-cicd

Copy link
Copy Markdown
Collaborator

PR_Github #84399 [ run ] triggered by Bot. Commit: cb6347c Link to invocation

@StanleySun639
StanleySun639 enabled auto-merge (squash) November 12, 2026 08:14

This branch has not been deployed

No deployments
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