Skip to content

[https://nvbugs/6160629][fix] AutoDeploy: Fix manual seed setting for standalone tests - #14954

Merged
galagam merged 3 commits into
NVIDIA:mainfrom
nv-auto-deploy:gagam/nvbug-6160629
Jun 5, 2026
Merged

[https://nvbugs/6160629][fix] AutoDeploy: Fix manual seed setting for standalone tests#14954
galagam merged 3 commits into
NVIDIA:mainfrom
nv-auto-deploy:gagam/nvbug-6160629

Conversation

@galagam

@galagam galagam commented Jun 4, 2026

Copy link
Copy Markdown
Collaborator

Summary by CodeRabbit

Description

BF16 with only 7 mantissa bits is more numerically sensitive than FP16. Test is passing for some seeds, failing for others with small mismatches for large values. The original atol/rtol = 1e-4/1e-4 threshold is too strict.
This test was manually seeded with a fixed seed at import, which produced deterministic results on normal runs. However, within the standalone package, it runs with pytest-xdist (dynamic test distribution), rendering the seed at the module ineffective.
This PR sets the same manual seed in each of the existing test.
This is not the correct solution from a numeric POV, since we're relying on some lucky seed where the accuracy can pass instead of correctly scoping and guarding the accuracy for any given input.
However, this practice is widely used in the test suite, and this PR aims to fix the standalone verification and not improve accuracy testing overall.

Examples:
seed=23
q_hf[(0, 3, 0, 129)] = -0.0810546875
q_flash/custom_q[...] = -0.0805664062
abs diff = 0.0004882812
relative diff ~= 0.0060

seed=41
k_hf[(1, 2, 0, 189)] = 0.7148437500
k_flash/custom_k[...] = 0.7187500000
abs diff = 0.0039062500
relative diff ~= 0.0054

seed=95
q_hf[(1, 3, 0, 227)] = 1.1640625000
q_flash/custom_q[...] = 1.1718750000
abs diff = 0.0078125000
relative diff ~= 0.0067

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.

@galagam
galagam requested a review from a team as a code owner June 4, 2026 10:43
@galagam
galagam requested a review from Fridah-nv June 4, 2026 10:43
@galagam galagam self-assigned this Jun 4, 2026
@coderabbitai

coderabbitai Bot commented Jun 4, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Walkthrough

Test tolerance for RoPE bfloat16 comparison is relaxed from rtol=1e-4 to rtol=1e-2 with an explanatory comment about potential unit-in-last-place differences between FlashInfer and HuggingFace implementations.

Changes

RoPE Test Tolerance

Layer / File(s) Summary
RoPE bfloat16 tolerance relaxation
tests/unittest/auto_deploy/singlegpu/custom_ops/rope/test_rope_op_variants.py
Relative tolerance (rtol) for the torch.bfloat16 case in test_flashinfer_custom_op_and_hf_impl is increased from 1e-4 to 1e-2, with an added comment noting that FlashInfer and HF reference implementations may differ by one bfloat16 ULP.

🎯 1 (Trivial) | ⏱️ ~2 minutes

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Title check ⚠️ Warning The PR title states 'Fix manual seed setting for standalone tests', but the actual change is about increasing numeric tolerance (rtol) for a BF16 HF vs FlashInfer rope test, which is unrelated to seed setting. Update the title to accurately reflect the main change, such as 'Increase rtol for bf16 HF vs FlashInfer rope test' or similar.
✅ Passed checks (4 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.
Description check ✅ Passed The PR description clearly explains the issue (BF16 numeric sensitivity), the solution (relaxed rtol from 1e-4 to 1e-2), rationale, and includes concrete examples with observed differences.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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

@galagam

galagam commented Jun 4, 2026

Copy link
Copy Markdown
Collaborator Author

/bot run

@tensorrt-cicd

Copy link
Copy Markdown
Collaborator

PR_Github #52051 [ run ] triggered by Bot. Commit: efd0aa5 Link to invocation

@tensorrt-cicd

Copy link
Copy Markdown
Collaborator

PR_Github #52051 [ run ] completed with state SUCCESS. Commit: efd0aa5
/LLM/main/L0_MergeRequest_PR pipeline #41385 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

@galagam

galagam commented Jun 4, 2026

Copy link
Copy Markdown
Collaborator Author

/bot run

@tensorrt-cicd

Copy link
Copy Markdown
Collaborator

PR_Github #52087 [ run ] triggered by Bot. Commit: efd0aa5 Link to invocation

@bmarimuthu-nv bmarimuthu-nv 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.

Thanks @galagam !

@galagam
galagam enabled auto-merge (squash) June 4, 2026 17:08
@galagam
galagam disabled auto-merge June 4, 2026 17:08
Comment thread tests/unittest/auto_deploy/singlegpu/custom_ops/rope/test_rope_op_variants.py Outdated
galagam added 2 commits June 4, 2026 10:20
…s FI rope test

BF16 with only 7 mantissa bits is more numerically sensitive than FP16.
Test is passing for some seeds, failing for others with small mismatches
for large values. The original atol/rtol = 1e-4/1e-4 threshold is too strict.
Increase to rtol to 1e-2 relaxes check for larger magnitudes.

Examples:
  seed=23
  q_hf[(0, 3, 0, 129)]      = -0.0810546875
  q_flash/custom_q[...]     = -0.0805664062
  abs diff                  = 0.0004882812
  relative diff             ~= 0.0060

  seed=41
  k_hf[(1, 2, 0, 189)]      = 0.7148437500
  k_flash/custom_k[...]     = 0.7187500000
  abs diff                  = 0.0039062500
  relative diff             ~= 0.0054

  seed=95
  q_hf[(1, 3, 0, 227)]      = 1.1640625000
  q_flash/custom_q[...]     = 1.1718750000
  abs diff                  = 0.0078125000
  relative diff             ~= 0.0067

Signed-off-by: Gal Hubara Agam <96368689+galagam@users.noreply.github.com>
Signed-off-by: Gal Hubara Agam <96368689+galagam@users.noreply.github.com>
@galagam
galagam force-pushed the gagam/nvbug-6160629 branch from 8669233 to bb009c2 Compare June 4, 2026 17:22
@galagam

galagam commented Jun 4, 2026

Copy link
Copy Markdown
Collaborator Author

/bot run

@tensorrt-cicd

Copy link
Copy Markdown
Collaborator

PR_Github #52126 [ run ] triggered by Bot. Commit: bb009c2 Link to invocation

@tensorrt-cicd

Copy link
Copy Markdown
Collaborator

PR_Github #52087 [ run ] completed with state ABORTED. Commit: efd0aa5

Link to invocation

@tensorrt-cicd

Copy link
Copy Markdown
Collaborator

PR_Github #52126 [ run ] completed with state SUCCESS. Commit: bb009c2
/LLM/main/L0_MergeRequest_PR pipeline #41452 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

The standalone-package test (test_run_unit_tests) runs the auto_deploy
unit tests under pytest-xdist (-n 4). test_rope_op_variants.py seeded the
RNG once at module import and never per test, so each test's inputs
depended on how many RNG draws the preceding tests consumed. Under xdist,
tests are distributed across workers dynamically, so that effective seed
varied run-to-run. The bf16/fp16 RoPE equivalence tests use near-ULP
tolerances that only hold for specific inputs, so the varying seed
produced ~3% intermittent failures (e.g.
test_flashinfer_custom_op_strided_interleaved[bfloat16-256]).

Seed the RNG explicitly at the start of each existing test so inputs are
independent of collection order, making the suite deterministic under
xdist. New tests should instead pick tolerances reflecting the dtype
error floor rather than relying on a fixed seed.

This also reverts the earlier rtol relaxation on
test_flashinfer_custom_op_and_hf_impl, which targeted a test that was not
the one failing and masked the precision gap.

Validated: full standalone suite passed 10/10 under pytest -n 4
(1719 passed, 0 failed each run).

Signed-off-by: Gal Hubara Agam <96368689+galagam@users.noreply.github.com>
@galagam

galagam commented Jun 5, 2026

Copy link
Copy Markdown
Collaborator Author

/bot run

@tensorrt-cicd

Copy link
Copy Markdown
Collaborator

PR_Github #52266 [ run ] triggered by Bot. Commit: 2630e83 Link to invocation

@galagam
galagam requested a review from bmarimuthu-nv June 5, 2026 08:27
@tensorrt-cicd

Copy link
Copy Markdown
Collaborator

PR_Github #52266 [ run ] completed with state SUCCESS. Commit: 2630e83
/LLM/main/L0_MergeRequest_PR pipeline #41576 completed with status: 'SUCCESS'

CI Report

Link to invocation

@galagam galagam changed the title [https://nvbugs/6160629][fix] AutoDeploy: increase rtol for bf16 HF vs FI rope test [https://nvbugs/6160629][fix] AutoDeploy: Fix manual seed setting for standalone tests Jun 5, 2026
@galagam
galagam merged commit 37ece3f into NVIDIA:main Jun 5, 2026
13 checks passed
fbxai pushed a commit to fbxai/TensorRT-LLM that referenced this pull request Jun 5, 2026
… standalone tests (NVIDIA#14954)

Signed-off-by: Gal Hubara Agam <96368689+galagam@users.noreply.github.com>
Signed-off-by: NVFB <186336021+NVFB@users.noreply.github.com>
2ez4bz pushed a commit to 2ez4bz/TensorRT-LLM that referenced this pull request Jun 8, 2026
… standalone tests (NVIDIA#14954)

Signed-off-by: Gal Hubara Agam <96368689+galagam@users.noreply.github.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