From b043da5dd969cf399deec690179b72daf0c97f20 Mon Sep 17 00:00:00 2001 From: Keval Morabia <28916987+kevalmorabia97@users.noreply.github.com> Date: Thu, 6 Aug 2026 10:00:44 -0700 Subject: [PATCH] Run example lanes only when the files they cover change A one-line edit to any example started all 12 example lanes. One changed-files pass now sets a flag per lane, and each lane gates on its own flag. Lane granularity, not per-example: a job's `if` cannot read `matrix`, so gating an individual example would need a job per example. The lane paths use brace expansion so an example is one word, not two patterns: "{examples,tests/examples}/{llm_qat,...}/**". The extra one-sided entries (dataset, hf_ptq, onnx_ptq) carry a comment saying which test reaches into them. Also: - Ignore docs-only changes (.md/.rst/.png/.ipynb) in the shared gate, so a README edit no longer starts GPU jobs in any of its callers. - Share the PR merge-base steps between the two gates as a composite action, inline the wait-for-checks step, and delete _wait_for_checks.yml. - Drop the DCO wait from unit tests: DCO can be marked passing manually, so blocking the matrix on it only delayed feedback. - Consolidate the two TensorRT-LLM lanes into one gated like the others, so llm_eval runs on PRs and not just nightly, and move gpt-oss there since its deploy step needs tensorrt_llm and silently skipped on the pytorch image. - Bump the torch and regression lanes to 26.07. - Detect deletions in every gate: any_changed is ACMR and drops deleted files, so a delete-only PR ran nothing. Use any_modified (ACMRD). - Watch the shared conftests: pytest loads tests/conftest.py and tests/examples/conftest.py for every suite, but no gate matched them, so editing a shared fixture ran nothing. - Exclude trufflehog's lob detector, whose (live|test)_[a-zA-Z0-9_]{35} pattern matches 60 pytest function names and reports them as verified secrets. Only nightly failed on it: schedule runs scan all history, PRs scan their diff. Co-Authored-By: Claude Opus 5 Signed-off-by: Keval Morabia <28916987+kevalmorabia97@users.noreply.github.com> --- .github/PULL_REQUEST_TEMPLATE.md | 2 +- .github/actions/pr-merge-base/action.yml | 38 ++++++ .github/workflows/_pr_gate.yml | 45 ++++--- .github/workflows/_wait_for_checks.yml | 29 ----- .github/workflows/code_quality.yml | 5 +- .github/workflows/example_tests.yml | 141 ++++++++++++++------- .github/workflows/gpu_tests.yml | 6 +- .github/workflows/regression_tests.yml | 6 +- .github/workflows/unit_tests.yml | 12 +- tests/examples/gpt-oss/test_gpt_oss_qat.py | 3 - 10 files changed, 176 insertions(+), 111 deletions(-) create mode 100644 .github/actions/pr-merge-base/action.yml delete mode 100644 .github/workflows/_wait_for_checks.yml diff --git a/.github/PULL_REQUEST_TEMPLATE.md b/.github/PULL_REQUEST_TEMPLATE.md index 0c46b0d3d66..a43f18c9bc1 100644 --- a/.github/PULL_REQUEST_TEMPLATE.md +++ b/.github/PULL_REQUEST_TEMPLATE.md @@ -22,7 +22,7 @@ Make sure you read and follow the [Security Best Practices](https://github.com/N - Is this change backward compatible?: ✅ / ❌ / N/A - If you copied code from any other sources or added a new PIP dependency, did you follow guidance in `CONTRIBUTING.md`: ✅ / ❌ / N/A - Did you write any new necessary tests?: ✅ / ❌ / N/A -- Did you update [Changelog](https://github.com/NVIDIA/Model-Optimizer/blob/main/CHANGELOG.rst)?: ✅ / ❌ / N/A +- Did you update [Changelog](https://github.com/NVIDIA/Model-Optimizer/blob/main/CHANGELOG.rst)?: ✅ / ❌ / N/A - Did you get Claude approval on this PR?: ✅ / ❌ / N/A ### Additional Information diff --git a/.github/actions/pr-merge-base/action.yml b/.github/actions/pr-merge-base/action.yml new file mode 100644 index 00000000000..cd4c1bc55df --- /dev/null +++ b/.github/actions/pr-merge-base/action.yml @@ -0,0 +1,38 @@ +name: PR merge base +description: > + Resolve the commit to diff a copied PR branch against: the merge base of the PR's head and its + target branch. Outputs are empty for non-PR triggers, which have no diff to inspect. + Requires a prior actions/checkout with fetch-depth 0. + +outputs: + merge_base: + description: Commit to use as the base for changed-file comparisons. + value: ${{ steps.calculate-merge-base.outputs.merge-base }} + head_sha: + description: The PR's head commit. + value: ${{ steps.pr-shas.outputs.head_sha }} + +runs: + using: composite + steps: + - if: startsWith(github.ref, 'refs/heads/pull-request/') + id: get-pr-info + uses: nv-gha-runners/get-pr-info@main + # Extract SHAs from pr-info JSON via shell to avoid fromJSON on potentially-empty outputs + - if: startsWith(github.ref, 'refs/heads/pull-request/') + id: pr-shas + shell: bash + env: + PR_INFO: ${{ steps.get-pr-info.outputs.pr-info }} + run: | + echo "head_sha=$(echo "$PR_INFO" | jq -r '.head.sha')" >> $GITHUB_OUTPUT + echo "base_sha=$(echo "$PR_INFO" | jq -r '.base.sha')" >> $GITHUB_OUTPUT + # Get commit from the target branch that is present in the PR to use as base for changed files + - if: startsWith(github.ref, 'refs/heads/pull-request/') + id: calculate-merge-base + shell: bash + run: | + # Assign first: piping git into tee would mask a merge-base failure behind tee's exit + # status and emit an empty base, silently changing which lanes run. + merge_base=$(git merge-base "${{ steps.pr-shas.outputs.base_sha }}" "${{ steps.pr-shas.outputs.head_sha }}") + echo "merge-base=$merge_base" | tee --append "${GITHUB_OUTPUT}" diff --git a/.github/workflows/_pr_gate.yml b/.github/workflows/_pr_gate.yml index d1e6dad0f9f..15c7a47a520 100644 --- a/.github/workflows/_pr_gate.yml +++ b/.github/workflows/_pr_gate.yml @@ -7,6 +7,15 @@ on: description: "Newline-separated list of file patterns to watch for changes" required: true type: string + files_ignore: + description: "Newline-separated patterns to ignore; defaults to docs-only files no test runs" + required: false + type: string + default: | + **.ipynb + **.md + **.png + **.rst outputs: any_changed: description: "Whether any relevant files changed" @@ -16,7 +25,7 @@ jobs: check-file-changes: runs-on: ubuntu-latest outputs: - any_changed: ${{ steps.changed-tests.outputs.any_changed || steps.non-pr.outputs.any_changed }} + any_changed: ${{ steps.changed-tests.outputs.any_modified || steps.non-pr.outputs.any_changed }} steps: # For non-PR triggers (schedule, workflow_dispatch), always run tests - id: non-pr @@ -27,39 +36,29 @@ jobs: with: fetch-depth: 0 - if: startsWith(github.ref, 'refs/heads/pull-request/') - id: get-pr-info - uses: nv-gha-runners/get-pr-info@main - # Extract SHAs from pr-info JSON via shell to avoid fromJSON on potentially-empty outputs - - if: startsWith(github.ref, 'refs/heads/pull-request/') - id: pr-shas - env: - PR_INFO: ${{ steps.get-pr-info.outputs.pr-info }} - run: | - echo "head_sha=$(echo "$PR_INFO" | jq -r '.head.sha')" >> $GITHUB_OUTPUT - echo "base_sha=$(echo "$PR_INFO" | jq -r '.base.sha')" >> $GITHUB_OUTPUT - # Get commit from main branch that is present in the PR to use as base for changed files - - if: startsWith(github.ref, 'refs/heads/pull-request/') - id: calculate-merge-base - run: | - (echo -n "merge-base="; git merge-base "${{ steps.pr-shas.outputs.base_sha }}" "${{ steps.pr-shas.outputs.head_sha }}") | tee --append "${GITHUB_OUTPUT}" + id: base + uses: ./.github/actions/pr-merge-base - if: startsWith(github.ref, 'refs/heads/pull-request/') name: Check for changes in test-relevant directories id: changed-tests uses: step-security/changed-files@v46.0.5 with: - base_sha: ${{ steps.calculate-merge-base.outputs.merge-base }} - sha: ${{ steps.pr-shas.outputs.head_sha }} + base_sha: ${{ steps.base.outputs.merge_base }} + sha: ${{ steps.base.outputs.head_sha }} files: ${{ inputs.files }} + files_ignore: ${{ inputs.files_ignore }} fail_on_initial_diff_error: true wait-checks: needs: [check-file-changes] if: >- startsWith(github.ref, 'refs/heads/pull-request/') && needs.check-file-changes.outputs.any_changed == 'true' - uses: ./.github/workflows/_wait_for_checks.yml + runs-on: ubuntu-latest permissions: checks: read - secrets: inherit - with: - match_pattern: "^linux$" # Wait for Unit tests / linux (DCO is a prerequisite of linux) - delay: 300s + steps: + - uses: poseidon/wait-for-status-checks@v0.6.0 + with: + token: ${{ secrets.GITHUB_TOKEN }} + match_pattern: "^linux$" # Wait for Unit tests / linux + delay: 300s diff --git a/.github/workflows/_wait_for_checks.yml b/.github/workflows/_wait_for_checks.yml deleted file mode 100644 index 9e28fcaa28a..00000000000 --- a/.github/workflows/_wait_for_checks.yml +++ /dev/null @@ -1,29 +0,0 @@ -name: Wait for checks - -on: - workflow_call: - inputs: - match_pattern: - required: true - type: string - delay: - required: false - type: string - default: 10s - -jobs: - wait: - runs-on: ubuntu-latest - permissions: - checks: read - steps: - - name: Wait for checks (PRs only) - if: github.event_name == 'pull_request' || startsWith(github.ref, 'refs/heads/pull-request/') - uses: poseidon/wait-for-status-checks@v0.6.0 - with: - token: ${{ secrets.GITHUB_TOKEN }} - match_pattern: ${{ inputs.match_pattern }} - delay: ${{ inputs.delay }} - - name: No-op for non-PR events - if: github.event_name != 'pull_request' && !startsWith(github.ref, 'refs/heads/pull-request/') - run: echo "Not a pull_request event" diff --git a/.github/workflows/code_quality.yml b/.github/workflows/code_quality.yml index db0fef07b6d..3a53f7ae356 100644 --- a/.github/workflows/code_quality.yml +++ b/.github/workflows/code_quality.yml @@ -24,6 +24,9 @@ jobs: - name: Secret Scanning uses: trufflesecurity/trufflehog@v3.90.5 with: - extra_args: --results=verified,unknown + # Exclude lob: its `(live|test)_[a-zA-Z0-9_]{35}` pattern matches any pytest function + # whose name happens to be exactly 35 characters after `test_`, and it reports them as + # verified. Only nightly sees them, since it scans all history while PRs scan their diff. + extra_args: --results=verified,unknown --exclude-detectors=lob - name: Run code quality checks run: pip install nox uv && nox -s pre_commit_all diff --git a/.github/workflows/example_tests.yml b/.github/workflows/example_tests.yml index f7e06262e1f..843e54f48dc 100644 --- a/.github/workflows/example_tests.yml +++ b/.github/workflows/example_tests.yml @@ -18,49 +18,121 @@ concurrency: # a comma-separated list of example names whose test failures should be non-blocking, e.g. "torch_trt,llm_qat" jobs: + # One changed-files pass decides which lanes run. Lane granularity, not per-example: a job's + # `if` cannot read `matrix`, so gating an individual example would need a job per example. pr-gate: - uses: ./.github/workflows/_pr_gate.yml + runs-on: ubuntu-latest permissions: checks: read - secrets: inherit - with: - files: | - .github/actions/cache-extensions/** - .github/workflows/_example_tests_runner.yml - .github/workflows/example_tests.yml - examples/** - modelopt/** - pyproject.toml - tests/examples/** + contents: read + outputs: + any_changed: ${{ steps.lanes.outputs.any_changed }} + torch: ${{ steps.lanes.outputs.torch }} + trtllm: ${{ steps.lanes.outputs.trtllm }} + megatron: ${{ steps.lanes.outputs.megatron }} + onnx: ${{ steps.lanes.outputs.onnx }} + steps: + - uses: actions/checkout@v6 + with: + fetch-depth: 0 + - if: startsWith(github.ref, 'refs/heads/pull-request/') + id: base + uses: ./.github/actions/pr-merge-base + - if: startsWith(github.ref, 'refs/heads/pull-request/') + id: changed + uses: step-security/changed-files@v46.0.5 + with: + base_sha: ${{ steps.base.outputs.merge_base }} + sha: ${{ steps.base.outputs.head_sha }} + fail_on_initial_diff_error: true + files_ignore: | + **.ipynb + **.md + **.png + **.rst + # `common` runs every lane. Each lane lists the example directories it covers, plus any + # extra directory those tests reach into (e.g. hf_ptq's script runs lm_eval from + # ../llm_eval, and the speculative_decoding test drives hf_ptq). + files_yaml: | + common: + - .github/actions/** + - .github/workflows/_example_tests_runner.yml + - .github/workflows/example_tests.yml + - modelopt/** + - modelopt_recipes/** + - pyproject.toml + - tests/_test_utils/** + - tests/conftest.py + - tests/examples/conftest.py + torch: + - "{examples,tests/examples}/{llm_distill,llm_qat,llm_sparsity,specdec_bench,speculative_decoding}/**" + - examples/dataset/** # data prep for llm_qat and speculative_decoding + - examples/hf_ptq/** # the speculative_decoding test drives hf_ptq + trtllm: + - "{examples,tests/examples}/{gpt-oss,hf_ptq,llm_eval}/**" + - examples/dataset/** # data prep for gpt-oss + megatron: + - "{examples,tests/examples}/megatron_bridge/**" + onnx: + - "{examples,tests/examples}/{diffusers,torch_onnx,torch_trt}/**" + - examples/onnx_ptq/** # torch_trt reuses onnx_ptq + - id: lanes + env: + # Nightly and on-demand runs have no diff to inspect, so they run everything. + NON_PR: ${{ !startsWith(github.ref, 'refs/heads/pull-request/') }} + COMMON: ${{ steps.changed.outputs.common_any_modified }} + TORCH: ${{ steps.changed.outputs.torch_any_modified }} + TRTLLM: ${{ steps.changed.outputs.trtllm_any_modified }} + MEGATRON: ${{ steps.changed.outputs.megatron_any_modified }} + ONNX: ${{ steps.changed.outputs.onnx_any_modified }} + run: | + any=false + for lane in TORCH TRTLLM MEGATRON ONNX; do + if [[ "$NON_PR" == "true" || "$COMMON" == "true" || "${!lane}" == "true" ]]; then + run=true + any=true + else + run=false + fi + echo "${lane,,}=$run" | tee --append "$GITHUB_OUTPUT" + done + echo "any_changed=$any" | tee --append "$GITHUB_OUTPUT" + - name: Wait for unit tests before spending GPU runners + if: ${{ steps.lanes.outputs.any_changed == 'true' && startsWith(github.ref, 'refs/heads/pull-request/') }} + uses: poseidon/wait-for-status-checks@v0.6.0 + with: + token: ${{ secrets.GITHUB_TOKEN }} + match_pattern: "^linux$" # Wait for Unit tests / linux + delay: 300s ##### PyTorch Example Tests ##### torch: needs: [pr-gate] - if: needs.pr-gate.outputs.any_changed == 'true' + if: needs.pr-gate.outputs.torch == 'true' strategy: fail-fast: false matrix: - example: [gpt-oss, llm_distill, llm_qat, llm_sparsity, specdec_bench, speculative_decoding] + example: [llm_distill, llm_qat, llm_sparsity, specdec_bench, speculative_decoding] uses: ./.github/workflows/_example_tests_runner.yml permissions: contents: read secrets: inherit with: - docker_image: "nvcr.io/nvidia/pytorch:26.06-py3" + docker_image: "nvcr.io/nvidia/pytorch:26.07-py3" example: ${{ matrix.example }} timeout_minutes: 30 pip_install_extras: "[hf,dev-test]" runner: ${{ startsWith(github.ref, 'refs/heads/pull-request/') && 'linux-amd64-gpu-rtxpro6000-latest-1' || 'linux-amd64-gpu-rtxpro6000-latest-2' }} allow_failure: ${{ contains(format(',{0},', vars.ALLOW_FAILURE_EXAMPLE_TESTS), format(',{0},', matrix.example)) }} - ##### TensorRT-LLM Example Tests (pr/non-pr split: non-pr runs extra eval examples) ##### - trtllm-pr: + ##### TensorRT-LLM Example Tests ##### + trtllm: needs: [pr-gate] - if: startsWith(github.ref, 'refs/heads/pull-request/') && needs.pr-gate.outputs.any_changed == 'true' + if: needs.pr-gate.outputs.trtllm == 'true' strategy: fail-fast: false matrix: - example: [hf_ptq] + example: [gpt-oss, hf_ptq, llm_eval] uses: ./.github/workflows/_example_tests_runner.yml permissions: contents: read @@ -69,30 +141,13 @@ jobs: docker_image: "nvcr.io/nvidia/tensorrt-llm/release:1.3.0rc20" example: ${{ matrix.example }} pip_install_extras: "[hf,dev-test]" - runner: linux-amd64-gpu-rtxpro6000-latest-1 - allow_failure: ${{ contains(format(',{0},', vars.ALLOW_FAILURE_EXAMPLE_TESTS), format(',{0},', matrix.example)) }} - - trtllm-non-pr: - if: ${{ !startsWith(github.ref, 'refs/heads/pull-request/') }} - strategy: - fail-fast: false - matrix: - example: [llm_eval, hf_ptq] - uses: ./.github/workflows/_example_tests_runner.yml - permissions: - contents: read - secrets: inherit - with: - docker_image: "nvcr.io/nvidia/tensorrt-llm/release:1.3.0rc20" - example: ${{ matrix.example }} - pip_install_extras: "[hf,dev-test]" - runner: linux-amd64-gpu-rtxpro6000-latest-2 + runner: ${{ startsWith(github.ref, 'refs/heads/pull-request/') && 'linux-amd64-gpu-rtxpro6000-latest-1' || 'linux-amd64-gpu-rtxpro6000-latest-2' }} allow_failure: ${{ contains(format(',{0},', vars.ALLOW_FAILURE_EXAMPLE_TESTS), format(',{0},', matrix.example)) }} ##### Megatron Example Tests ##### megatron: needs: [pr-gate] - if: needs.pr-gate.outputs.any_changed == 'true' + if: needs.pr-gate.outputs.megatron == 'true' uses: ./.github/workflows/_example_tests_runner.yml permissions: contents: read @@ -108,7 +163,7 @@ jobs: ##### ONNX/TensorRT Example Tests ##### onnx: needs: [pr-gate] - if: needs.pr-gate.outputs.any_changed == 'true' + if: needs.pr-gate.outputs.onnx == 'true' strategy: fail-fast: false matrix: @@ -132,16 +187,16 @@ jobs: example-pr-required-check: # Run even if example tests are skipped if: ${{ startsWith(github.ref, 'refs/heads/pull-request/') && always() }} - needs: [pr-gate, torch, trtllm-pr, megatron, onnx] + needs: [pr-gate, torch, trtllm, megatron, onnx] runs-on: ubuntu-latest steps: - name: Required example tests did not succeed if: | needs.pr-gate.result != 'success' || (needs.pr-gate.outputs.any_changed == 'true' && ( - needs.torch.result != 'success' || - needs.trtllm-pr.result != 'success' || - needs.megatron.result != 'success' || - needs.onnx.result != 'success' + !contains(fromJSON('["success", "skipped"]'), needs.torch.result) || + !contains(fromJSON('["success", "skipped"]'), needs.trtllm.result) || + !contains(fromJSON('["success", "skipped"]'), needs.megatron.result) || + !contains(fromJSON('["success", "skipped"]'), needs.onnx.result) )) run: exit 1 diff --git a/.github/workflows/gpu_tests.yml b/.github/workflows/gpu_tests.yml index 49e538aab5f..3b226a73926 100644 --- a/.github/workflows/gpu_tests.yml +++ b/.github/workflows/gpu_tests.yml @@ -22,11 +22,15 @@ jobs: secrets: inherit with: files: | - .github/actions/cache-extensions/** + .github/actions/** + .github/workflows/_pr_gate.yml .github/workflows/gpu_tests.yml modelopt/** + modelopt_recipes/** noxfile.py pyproject.toml + tests/_test_utils/** + tests/conftest.py tests/gpu/** tests/gpu_megatron/** tests/gpu_trtllm/** diff --git a/.github/workflows/regression_tests.yml b/.github/workflows/regression_tests.yml index a8fa5e2917a..cbf37fa0522 100644 --- a/.github/workflows/regression_tests.yml +++ b/.github/workflows/regression_tests.yml @@ -23,10 +23,14 @@ jobs: secrets: inherit with: files: | + .github/actions/** + .github/workflows/_pr_gate.yml .github/workflows/regression_tests.yml modelopt/torch/** noxfile.py pyproject.toml + tests/_test_utils/** + tests/conftest.py tests/regression/** examples/speculative_decoding/** examples/dataset/** @@ -39,7 +43,7 @@ jobs: runs-on: ${{ startsWith(github.ref, 'refs/heads/pull-request/') && 'linux-amd64-gpu-rtxpro6000-latest-1' || 'linux-amd64-gpu-rtxpro6000-latest-2' }} timeout-minutes: 15 container: - image: nvcr.io/nvidia/pytorch:26.01-py3 + image: nvcr.io/nvidia/pytorch:26.07-py3 env: GIT_DEPTH: 1000 # For correct version PIP_CONSTRAINT: "" # Disable pip constraint for upgrading packages diff --git a/.github/workflows/unit_tests.yml b/.github/workflows/unit_tests.yml index 8b7f0499703..e2f5dd91c40 100644 --- a/.github/workflows/unit_tests.yml +++ b/.github/workflows/unit_tests.yml @@ -26,17 +26,10 @@ concurrency: cancel-in-progress: true jobs: - check-dco: - uses: ./.github/workflows/_wait_for_checks.yml - permissions: - checks: read - secrets: inherit - with: - match_pattern: "^DCO$" check-file-changes: runs-on: ubuntu-latest outputs: - any_changed: ${{ steps.changed.outputs.any_changed || steps.non-pr.outputs.any_changed }} + any_changed: ${{ steps.changed.outputs.any_modified || steps.non-pr.outputs.any_changed }} steps: - id: non-pr if: github.event_name == 'schedule' || github.event_name == 'workflow_dispatch' @@ -53,15 +46,16 @@ jobs: files: | .github/workflows/unit_tests.yml modelopt/** + modelopt_recipes/** noxfile.py pyproject.toml tests/unit/** + tests/_test_utils/** tools/launcher/** tools/mcp/** tools/resource_monitor.py .agents/skills/** linux: - needs: [check-dco] runs-on: ubuntu-latest timeout-minutes: 15 steps: diff --git a/tests/examples/gpt-oss/test_gpt_oss_qat.py b/tests/examples/gpt-oss/test_gpt_oss_qat.py index 69069b33ccb..f584f7b7f34 100644 --- a/tests/examples/gpt-oss/test_gpt_oss_qat.py +++ b/tests/examples/gpt-oss/test_gpt_oss_qat.py @@ -224,9 +224,6 @@ def deploy_gpt_oss_trtllm(self, tmp_path, model_path_override=None): model_path_override: Optional path to the model to deploy (e.g., MXFP4 checkpoint). If None, uses self.model_path """ - # Skip if tensorrt_llm is not available - pytest.importorskip("tensorrt_llm") - # Use override path if provided, otherwise use original model path deploy_model_path = model_path_override or self.model_path