chore: move python pin to <=3.14 - #664
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughThe project now supports Python 3.14, updates CPU and CUDA dependency versions and uv indexes, and aligns container, documentation, release, and Slurm installation commands with the new vLLM and FlashInfer wheel locations. ChangesPython and dependency updates
Estimated code review effort: 2 (Simple) | ~15 minutes Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
25c4500 to
a5fb421
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 9a5d1a99-d348-4442-ba8a-4c00bce35378
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock,!uv.lock
📒 Files selected for processing (1)
pyproject.toml
📜 Review details
⏰ Context from checks skipped due to timeout. (10)
- GitHub Check: Unit Tests (3.11)
- GitHub Check: End-user Wheel Install
- GitHub Check: Unit Tests (3.13)
- GitHub Check: Unit Tests (3.12)
- GitHub Check: Smoke Tests
- GitHub Check: Greptile Review
- GitHub Check: Typecheck
- GitHub Check: Analyze (Python)
- GitHub Check: Analyze (python)
- GitHub Check: Analyze (javascript-typescript)
🧰 Additional context used
📓 Path-based instructions (3)
pyproject.toml
📄 CodeRabbit inference engine (.cursor/rules/repo-navigation.mdc)
Configure package metadata, dependencies, extras (cpu/cu129/engine), and uv configuration in
pyproject.tomlRun the lock-drift check when changing Python dependencies or
pyproject.toml; keepuv.locksynchronized.
Files:
pyproject.toml
⚙️ CodeRabbit configuration file
Treat pyproject.toml as high-risk. Check package metadata, uv indexes, dependency groups, optional extras, Python version bounds, hatch config, ty config, script entry points, dependency consistency, and whether changes require regenerating uv.lock.
Files:
pyproject.toml
**/*.toml
📄 CodeRabbit inference engine (STYLE_GUIDE.md)
Use spaces around
=, comment dependency pins inline, and follow the prescribedpyproject.tomlsection order.
Files:
pyproject.toml
**/*
📄 CodeRabbit inference engine (STYLE_GUIDE.md)
**/*: Every source file requires the SPDX copyright and license header appropriate to its file format.
End files with a newline, remove trailing whitespace, use one space between sentences, and keep code, comments, and docstrings within 120 characters.
**/*: All contributions must use verified Git commits and DCO sign-off; unsigned or unsigned-off commits cannot be merged.
Branches other thanmainmust follow<author>/<description>, optionally including an issue ID or type; branch names must use lowercase alphanumeric characters and hyphens.
Commits merged tomainmust follow Conventional Commits, using a valid lowercase type and a description of at most 100 characters.
Files:
pyproject.toml
⚙️ CodeRabbit configuration file
**/*: Review as a senior maintainer for NeMo Safe Synthesizer. Prioritize issues that can change behavior, break user workflows, weaken privacy guarantees, hide failures, make tests unreliable, or create maintenance risk. Avoid generic style commentary unless it points to a concrete project convention that automated tools will not catch.
Comment only when the finding is actionable and tied to changed code. For each finding, state the impact, the condition that triggers it, and the smallest practical fix. Prefer one precise comment over broad advice. Do not ask for refactors outside the PR scope unless the changed code creates the problem.
Review type guidance: - Potential issue: use for correctness bugs, data loss, privacy leaks,
security risks, broken public APIs, invalid config behavior, missing
validation, hidden failures, nondeterministic tests, or CI breakage.
- Refactor suggestion: use for local maintainability problems introduced
by the diff when they have clear future cost, such as duplicated setup,
unclear boundaries, over-mocking, avoidable complexity, or opaque test
helpers.- Nitpick: avoid in chill mode. Do not emit formatting, import-order,
wording, or style-only comments unless automated tools cannot catch the
issue and it affects maintainability.Severity guidance: - Critical: security/privacy leaks, data loss, training/test/holdout
contamination, or broken release/package/core pipeline execution.
- Major: incorrect generation/training/evaluation behavior, broken
CLI/SDK public API, invalid config defaults or validators, or GPU/vLLM
cleanup and process-isolation bugs likely to fail CI or production
runs.- Minor: localized bugs, missing focused tests for changed behavior, or
bad test patterns that weaken regression coverage.- Trivial: small cleanup with no behavior impact. Usually suppress in
chill mode.- Info: context only. Avoid unless it helps reviewers understand risk.
Safe-Synthesizer-specific review focus: - Data ...
Files:
pyproject.toml
🔇 Additional comments (1)
pyproject.toml (1)
7-7: 🗄️ Data Integrity & IntegrationRun the required lock-drift check for this metadata change.
Changing
pyproject.tomlcan change the resolver’s supported-environment metadata. Verify the repository’s lock-drift check passes and thatuv.lockremains synchronized before merging.Sources: Coding guidelines, Path instructions
Greptile SummaryThis PR extends Python support from
Confidence Score: 5/5The changes are mechanical version bumps and URL updates applied consistently across all install scripts, Dockerfiles, and documentation; no correctness issues were found. All install commands, index URLs, and version pins are updated uniformly across every changed file. The CPU Dockerfile install gains the required flashinfer-cubin index, the CUDA dry-run gains both flashinfer indexes, and the CI matrix correctly adds Python 3.14. The only findings are a minor style inconsistency in one notebook and a gap in test assertions for the new flashinfer-cubin source — neither affects runtime behavior. Files Needing Attention: tests/test_gen_cuda_deps.py — the new flashinfer-cubin source and index added by the generator have no corresponding assertions in the updated test. Important Files Changed
|
coderabbitai
Bot
added
feature
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
zywind
left a comment
zywind
left a comment
There was a problem hiding this comment.
The new Python 3.14 job only exercises bootstrap-nss cpu, while .github/workflows/gpu-tests.yml still runs the GPU smoke suite exclusively on Python 3.13. Since GPU training and generation are the primary supported runtime workflows, the package metadata would advertise Python 3.14 support without testing those workflows on that interpreter. Please add Python 3.14 GPU smoke coverage (at minimum) before declaring general 3.14 support.
|
I reproduced this on dev blue at the PR head with Python 3.14.3. The CPU profile resolves, but the supported CUDA profile fails: UV_PROJECT_ENVIRONMENT=/tmp/ss-pr664-py314-cu129 \
uv sync --frozen --python 3.14 --extra cu129 --extra engine --group devThe old troubleshooting explanation is stale—vLLM itself now has a compatible |
|
yeah kendrick shared it in a DM, I think it's kinda SOL (not speed of light) until vLLM releases this guy vllm-project/vllm#48155 |
a5fb421 to
02faec2
Compare
coderabbitai
Bot
added
chore
There was a problem hiding this comment.
🧹 Nitpick comments (1)
pyproject.toml (1)
160-162: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winKeep the dependency rationale inline.
Move the comment onto the
openaipin, matching line 134, so it remains attached if the extra is reordered.Suggested fix
- # pulled in by vLLM but pinned too low, see https://github.com/vllm-project/vllm/issues/49103 - "openai>=2.25.0", + "openai>=2.25.0", # pulled in by vLLM but pinned too low, see https://github.com/vllm-project/vllm/issues/49103As per coding guidelines, TOML dependency pins must have rationale comments inline.
Source: Coding guidelines
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 7db2b020-cad3-4877-b542-bf07529f4f11
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock,!uv.lock
📒 Files selected for processing (2)
.github/workflows/ci-checks.ymlpyproject.toml
🚧 Files skipped from review as they are similar to previous changes (1)
- .github/workflows/ci-checks.yml
📜 Review details
⏰ Context from checks skipped due to timeout. (10)
- GitHub Check: Unit Tests (3.13)
- GitHub Check: Unit Tests (3.11)
- GitHub Check: Unit Tests (3.14)
- GitHub Check: Unit Tests (3.12)
- GitHub Check: Smoke Tests
- GitHub Check: End-user Wheel Install
- GitHub Check: Greptile Review
- GitHub Check: Typecheck
- GitHub Check: Analyze (Python)
- GitHub Check: Analyze (python)
🧰 Additional context used
📓 Path-based instructions (3)
pyproject.toml
📄 CodeRabbit inference engine (.cursor/rules/repo-navigation.mdc)
Configure package metadata, dependencies, extras (cpu/cu129/engine), and uv configuration in
pyproject.tomlRun the lock-drift check when changing Python dependencies or
pyproject.toml; keepuv.locksynchronized.
Files:
pyproject.toml
⚙️ CodeRabbit configuration file
Treat pyproject.toml as high-risk. Check package metadata, uv indexes, dependency groups, optional extras, Python version bounds, hatch config, ty config, script entry points, dependency consistency, and whether changes require regenerating uv.lock.
Files:
pyproject.toml
**/*.toml
📄 CodeRabbit inference engine (STYLE_GUIDE.md)
Use spaces around
=, comment dependency pins inline, and follow the prescribedpyproject.tomlsection order.
Files:
pyproject.toml
**/*
📄 CodeRabbit inference engine (STYLE_GUIDE.md)
**/*: Every source file requires the SPDX copyright and license header appropriate to its file format.
End files with a newline, remove trailing whitespace, use one space between sentences, and keep code, comments, and docstrings within 120 characters.
**/*: All contributions must use verified Git commits and DCO sign-off; unsigned or unsigned-off commits cannot be merged.
Branches other thanmainmust follow<author>/<description>, optionally including an issue ID or type; branch names must use lowercase alphanumeric characters and hyphens.
Commits merged tomainmust follow Conventional Commits, using a valid lowercase type and a description of at most 100 characters.
Files:
pyproject.toml
⚙️ CodeRabbit configuration file
**/*: Review as a senior maintainer for NeMo Safe Synthesizer. Prioritize issues that can change behavior, break user workflows, weaken privacy guarantees, hide failures, make tests unreliable, or create maintenance risk. Avoid generic style commentary unless it points to a concrete project convention that automated tools will not catch.
Comment only when the finding is actionable and tied to changed code. For each finding, state the impact, the condition that triggers it, and the smallest practical fix. Prefer one precise comment over broad advice. Do not ask for refactors outside the PR scope unless the changed code creates the problem.
Review type guidance: - Potential issue: use for correctness bugs, data loss, privacy leaks,
security risks, broken public APIs, invalid config behavior, missing
validation, hidden failures, nondeterministic tests, or CI breakage.
- Refactor suggestion: use for local maintainability problems introduced
by the diff when they have clear future cost, such as duplicated setup,
unclear boundaries, over-mocking, avoidable complexity, or opaque test
helpers.- Nitpick: avoid in chill mode. Do not emit formatting, import-order,
wording, or style-only comments unless automated tools cannot catch the
issue and it affects maintainability.Severity guidance: - Critical: security/privacy leaks, data loss, training/test/holdout
contamination, or broken release/package/core pipeline execution.
- Major: incorrect generation/training/evaluation behavior, broken
CLI/SDK public API, invalid config defaults or validators, or GPU/vLLM
cleanup and process-isolation bugs likely to fail CI or production
runs.- Minor: localized bugs, missing focused tests for changed behavior, or
bad test patterns that weaken regression coverage.- Trivial: small cleanup with no behavior impact. Usually suppress in
chill mode.- Info: context only. Avoid unless it helps reviewers understand risk.
Safe-Synthesizer-specific review focus: - Data ...
Files:
pyproject.toml
🔇 Additional comments (2)
pyproject.toml (2)
22-22: LGTM!Also applies to: 32-32, 93-93, 117-118, 134-137, 142-144, 251-253, 320-327
7-7: 🩺 Stability & AvailabilityKeep the
cu129Python 3.14 lock check before merging. If the locked stack does not resolve cleanly on 3.14, keeprequires-pythonbelow 3.14 until the CUDA dependency chain is updated.
02faec2 to
63bda53
Compare
63bda53 to
cfa20b2
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: a2d51b97-457a-41fb-b31d-7097a0774bf7
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock,!uv.lock
📒 Files selected for processing (11)
.github/workflows/ci-checks.ymlCONTRIBUTING.mdREADME.mdcontainers/Dockerfile.test_cidocs/dev-notes/posts/introducing-nemo-safe-synthesizer.mddocs/tutorials/differential-privacy.ipynbdocs/tutorials/safe-synthesizer-101.ipynbdocs/tutorials/time-series-financial-transactions.ipynbdocs/user-guide/getting-started.mdpyproject.tomlscript/slurm/slurm_nss_matrix.sh
🚧 Files skipped from review as they are similar to previous changes (4)
- containers/Dockerfile.test_ci
- .github/workflows/ci-checks.yml
- docs/user-guide/getting-started.md
- pyproject.toml
📜 Review details
⏰ Context from checks skipped due to timeout. (8)
- GitHub Check: Unit Tests (3.13)
- GitHub Check: Unit Tests (3.11)
- GitHub Check: Unit Tests (3.12)
- GitHub Check: Unit Tests (3.14)
- GitHub Check: Smoke Tests
- GitHub Check: End-user Wheel Install
- GitHub Check: Greptile Review
- GitHub Check: Analyze (Python)
🧰 Additional context used
📓 Path-based instructions (11)
**/*.{md,markdown,py}
📄 CodeRabbit inference engine (.cursor/rules/agent-markdown-style.mdc)
**/*.{md,markdown,py}: Avoid decorative bold (**text**) in list items, body text, and docstrings; use structural cues (headers, list markers, colons, backticks) for emphasis instead
Use backticks for code identifiers, paths, and CLI commands in markdown and docstrings
Files:
docs/dev-notes/posts/introducing-nemo-safe-synthesizer.mdREADME.mdCONTRIBUTING.md
**/*.{md,markdown}
📄 CodeRabbit inference engine (.cursor/rules/agent-markdown-style.mdc)
**/*.{md,markdown}: Bold is acceptable only in markdown tables where it's the conventional way to mark header-like cells in the body
Use##headers to segment markdown sections instead of bold text
Use--(em-dash) instead of-(hyphen) for asides in markdown
Files:
docs/dev-notes/posts/introducing-nemo-safe-synthesizer.mdREADME.mdCONTRIBUTING.md
docs/**/*.md
📄 CodeRabbit inference engine (.cursor/rules/writing-docs.mdc)
docs/**/*.md: Use MkDocs Material admonition syntax (!!! note, !!! warning, ??? tip) for highlighting important information and collapsible sections in documentation
Use MkDocs Material tabs syntax (=== "Label") to present alternative views or language-specific examples in documentation
Use code block syntax with title and highlight line parameters (title="filename", hl_lines="2 3") for code examples in documentation
Use Mermaid diagram syntax (```mermaid flowchart, etc.) for visualizations in documentationClassify documentation using Diátaxis and use MkDocs Material syntax for admonitions, tabs, and titled or highlighted code blocks.
docs/**/*.md: Documentation pages must be placed under the appropriate Diátaxis directory:getting-started,user-guide,architecture,reference, ordev-notes.
When adding or editing a documentation page, add it to thenav:section ofmkdocs.yml.
Use the configured MkDocs Material Markdown features correctly, including admonitions, content tabs, fenced code blocks, Mermaid diagrams, task lists, footnotes, definition lists, and emoji.
Files:
docs/dev-notes/posts/introducing-nemo-safe-synthesizer.md
**/*.md
📄 CodeRabbit inference engine (STYLE_GUIDE.md)
Do not use decorative bold in Markdown body text, list items, or docstrings; use single backticks for code identifiers, paths, and commands.
Files:
docs/dev-notes/posts/introducing-nemo-safe-synthesizer.mdREADME.mdCONTRIBUTING.md
**/*
📄 CodeRabbit inference engine (STYLE_GUIDE.md)
**/*: Every source file requires the SPDX copyright and license header appropriate to its file format.
End files with a newline, remove trailing whitespace, use one space between sentences, and keep code, comments, and docstrings within 120 characters.
**/*: All contributions must use DCO sign-off, adding aSigned-off-bytrailer to commit messages.
All commits merged tomainmust follow Conventional Commits syntax, with a lowercase valid type, optional scope, description of at most 100 characters, and!for breaking changes.
Commits merged tomainmust be cryptographically signed because the branch requires verified signatures.
Branches other thanmainmust follow<author>/<description>, optionally including an issue ID or valid type, using lowercase alphanumeric and hyphenated names.
Before submitting a pull request, runmise run format,mise run check, andmise run test; use mise tasks rather than invoking ruff or ty directly.
Pull requests should be squash-merged only after required approvals, CI checks, conversation resolution, and code-owner review are satisfied.
Never move a published release tag; if code changes after a candidate release, create and validate the nextrcNtag.
The stable release tag must point to the same tested commit SHA as the validated release candidate.
Files:
docs/dev-notes/posts/introducing-nemo-safe-synthesizer.mddocs/tutorials/differential-privacy.ipynbscript/slurm/slurm_nss_matrix.shREADME.mdCONTRIBUTING.mddocs/tutorials/time-series-financial-transactions.ipynbdocs/tutorials/safe-synthesizer-101.ipynb
⚙️ CodeRabbit configuration file
**/*: Review as a senior maintainer for NeMo Safe Synthesizer. Prioritize issues that can change behavior, break user workflows, weaken privacy guarantees, hide failures, make tests unreliable, or create maintenance risk. Avoid generic style commentary unless it points to a concrete project convention that automated tools will not catch.
Comment only when the finding is actionable and tied to changed code. For each finding, state the impact, the condition that triggers it, and the smallest practical fix. Prefer one precise comment over broad advice. Do not ask for refactors outside the PR scope unless the changed code creates the problem.
Review type guidance: - Potential issue: use for correctness bugs, data loss, privacy leaks,
security risks, broken public APIs, invalid config behavior, missing
validation, hidden failures, nondeterministic tests, or CI breakage.
- Refactor suggestion: use for local maintainability problems introduced
by the diff when they have clear future cost, such as duplicated setup,
unclear boundaries, over-mocking, avoidable complexity, or opaque test
helpers.- Nitpick: avoid in chill mode. Do not emit formatting, import-order,
wording, or style-only comments unless automated tools cannot catch the
issue and it affects maintainability.Severity guidance: - Critical: security/privacy leaks, data loss, training/test/holdout
contamination, or broken release/package/core pipeline execution.
- Major: incorrect generation/training/evaluation behavior, broken
CLI/SDK public API, invalid config defaults or validators, or GPU/vLLM
cleanup and process-isolation bugs likely to fail CI or production
runs.- Minor: localized bugs, missing focused tests for changed behavior, or
bad test patterns that weaken regression coverage.- Trivial: small cleanup with no behavior impact. Usually suppress in
chill mode.- Info: context only. Avoid unless it helps reviewers understand risk.
Safe-Synthesizer-specific review focus: - Data ...
Files:
docs/dev-notes/posts/introducing-nemo-safe-synthesizer.mddocs/tutorials/differential-privacy.ipynbscript/slurm/slurm_nss_matrix.shREADME.mdCONTRIBUTING.mddocs/tutorials/time-series-financial-transactions.ipynbdocs/tutorials/safe-synthesizer-101.ipynb
**/*.{py,sh,yaml,yml,md}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
All Python, shell, YAML, YML, and Markdown source files must include SPDX copyright headers.
Files:
docs/dev-notes/posts/introducing-nemo-safe-synthesizer.mdscript/slurm/slurm_nss_matrix.shREADME.mdCONTRIBUTING.md
docs/**
⚙️ CodeRabbit configuration file
Review documentation as MkDocs Material content. Check Diataxis fit, accurate commands, internal links, code fences, and markdown style from STYLE_GUIDE.md.
Files:
docs/dev-notes/posts/introducing-nemo-safe-synthesizer.mddocs/tutorials/differential-privacy.ipynbdocs/tutorials/time-series-financial-transactions.ipynbdocs/tutorials/safe-synthesizer-101.ipynb
**/*.{sh,bash}
📄 CodeRabbit inference engine (AGENTS.md)
Never use
~inside double-quoted strings in shell scripts -- use$HOMEor an absolute path instead
Files:
script/slurm/slurm_nss_matrix.sh
script/**
⚙️ CodeRabbit configuration file
Review standalone scripts for reproducibility and operational safety. Check argument validation, quoting, repo-root detection, environment variables, generated artifacts, external commands, GPU/cluster assumptions, and whether the script should be wired through Makefile or documented in README/docs.
Files:
script/slurm/slurm_nss_matrix.sh
**/*.sh
⚙️ CodeRabbit configuration file
Review shell scripts for #!/usr/bin/env bash, set -euo pipefail where appropriate, quoting, repo root detection, and shellcheck compliance.
Files:
script/slurm/slurm_nss_matrix.sh
README.md
⚙️ CodeRabbit configuration file
Treat README.md as the project overview. Check that setup, usage, and links stay consistent with CONTRIBUTING.md, Makefile, and docs/.
Files:
README.md
🔇 Additional comments (6)
script/slurm/slurm_nss_matrix.sh (1)
125-130: LGTM!README.md (1)
23-26: LGTM!Also applies to: 28-33
docs/dev-notes/posts/introducing-nemo-safe-synthesizer.md (1)
90-91: LGTM!docs/tutorials/safe-synthesizer-101.ipynb (1)
54-66: LGTM!docs/tutorials/time-series-financial-transactions.ipynb (1)
60-71: LGTM!CONTRIBUTING.md (1)
788-790: 🩺 Stability & AvailabilityKeep the smoke command on Python 3.13.
CONTRIBUTING.mdandpyproject.tomlstill define support as 3.11–3.13 (requires-python = ">=3.11, <3.15"), so this line already matches the supported installation target.> Likely an incorrect or invalid review comment.
| "cell_type": "code", | ||
| "execution_count": null, | ||
| "id": "d501f043", | ||
| "metadata": {}, | ||
| "metadata": { | ||
| "vscode": { | ||
| "languageId": "plaintext" | ||
| } | ||
| }, | ||
| "outputs": [], |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Restore the installation introduction as a Markdown cell.
This cell is now declared as code, but its source is Markdown prose without %%bash. Executing it runs the explanatory sentence as Python and fails before users reach the installation cell. Restore "cell_type": "markdown" and remove the code-only execution metadata.
As per path instructions, prioritize issues that break user workflows; this cell prevents the tutorial from running as documented.
Proposed notebook metadata fix
- "cell_type": "code",
- "execution_count": null,
+ "cell_type": "markdown",
"id": "d501f043",
- "metadata": {
- "vscode": {
- "languageId": "plaintext"
- }
- },
- "outputs": [],
+ "metadata": {},📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| "cell_type": "code", | |
| "execution_count": null, | |
| "id": "d501f043", | |
| "metadata": {}, | |
| "metadata": { | |
| "vscode": { | |
| "languageId": "plaintext" | |
| } | |
| }, | |
| "outputs": [], | |
| "cell_type": "markdown", | |
| "id": "d501f043", | |
| "metadata": {}, |
Source: Path instructions
cfa20b2 to
ed2dd20
Compare
Signed-off-by: mkornfield <mkornfield@nvidia.com>
ed2dd20 to
4dce166
Compare
mckornfield
changed the title
zywind
left a comment
zywind
left a comment
There was a problem hiding this comment.
Local review found four issues to address before approval.
Signed-off-by: mkornfield <mkornfield@nvidia.com>
…NeMo#705) # Summary Fixes GPU smoke + e2e failing on `main` with: ``` × No solution found when resolving dependencies: ╰─▶ Because there is no version of flashinfer-cubin{sys_platform == 'linux'}==0.6.14 ... conclude that your requirements are unsatisfiable. ``` ## Root cause `setup-gpu-test-env` infers which indexes to pass to `uv` by matching the CUDA extra against each index's name or URL: ```python if index["name"].endswith(f"-{cuda_extra}") or f"/{cuda_extra}" in index["url"] ``` When that inference landed (NVIDIA-NeMo#691, Aug 4 16:50 UTC) every index encoded its variant one of those two ways, so the rule held. NVIDIA-NeMo#664 landed 27 minutes later (17:17 UTC) and added the first index that doesn't: `flashinfer-cubin`, which serves a single variant-agnostic `https://flashinfer.ai/whl/` and is bound to packages only through `[tool.uv.sources]`. It matches neither branch of the rule, so it was silently dropped — and since the install also passes `--no-sources`, the sources table couldn't reach it either. Both PRs were green on their own base; the breakage exists only in the merge. That index is the only place `flashinfer-cubin==0.6.14` exists: | Index | max `flashinfer-cubin` | | ----- | ---------------------- | | `https://flashinfer.ai/whl/` | 0.6.16.post1 (has 0.6.14) | | PyPI | 0.6.13 | | `https://flashinfer.ai/whl/cu129` | does not serve the package | | `https://wheels.vllm.ai/0.26.0/cu129` | 0.6.13 | ## Fix Also collect indexes referenced by `[tool.uv.sources]` for the target extra. Before / after, run against `pyproject.toml` at `cu129`: ``` https://download.pytorch.org/whl/cu129 https://flashinfer.ai/whl/cu129 + https://flashinfer.ai/whl/ https://wheels.vllm.ai/0.26.0/cu129 ``` This matches the index set in the documented manual install command, which is why installing by hand works today while CI does not. ## Verification Resolution flips with exactly that one index, reproducing CI's error before and succeeding after: ``` $ uv pip install --dry-run --no-deps --default-index https://pypi.org/simple \ --index .../cu129 (x3) --index-strategy unsafe-best-match flashinfer-cubin==0.6.14 × No solution found ... there is no version of flashinfer-cubin==0.6.14 $ # same, plus --index https://flashinfer.ai/whl/ Resolved 1 package + flashinfer-cubin==0.6.14 ``` `mise run format-check` passes. No dependency versions change. ## Pre-Review Checklist - [x] `mise run format && mise run check` - [x] GPU CI status check passes -- this PR is the fix for it ## Other Notes CI-only change; no source or dependency changes. Worth noting the `index_count < 3` guard did not catch this — the count stayed at exactly 3 while the wrong index was dropped. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Improved GPU test environment setup to reliably discover all CUDA package indexes configured for the selected extra. * Added support for indexes declared through source configuration, alongside existing name- and URL-based matching. <!-- end of auto-generated comment: release notes by coderabbit.ai --> Signed-off-by: Yunfeng Zhang <yunzhang@nvidia.com>
2 participants
Summary
Pre-Review Checklist
Ensure that the following pass:
mise run format && mise run checkor via prek validation.mise run testpasses locallymise run test:e2epasses locallymise run test:ci-containerpasses locally (recommended)/syncon this PR to trigger a run (auto-triggers on ready-for-review)Pre-Merge Checklist
Other Notes
Summary by CodeRabbit
openai>=2.25.0to relevant optional dependency groups.