Remove unused diffusers/cache_diffusion/pipeline and cuda-python dependency#996
Conversation
…ndency Signed-off-by: Keval Morabia <28916987+kevalmorabia97@users.noreply.github.com>
|
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:
📝 WalkthroughWalkthroughRemoves the cache_diffusion ONNX/TensorRT deployment stack and related artifacts: ONNX export config, TensorRT deploy/runtime code, SD3/SDXL forward helpers, benchmark script and its test, and Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (2 inconclusive)
✅ Passed checks (3 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #996 +/- ##
=======================================
Coverage 72.13% 72.13%
=======================================
Files 209 209
Lines 23631 23631
=======================================
Hits 17046 17046
Misses 6585 6585 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Signed-off-by: Keval Morabia <28916987+kevalmorabia97@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@CONTRIBUTING.md`:
- Line 46: Update the sentence in CONTRIBUTING.md that reads "If its not a
permissive license (e.g. MIT, Apache 2), you need to provide a justification for
the use of the dependency in the PR and wait check with
`@NVIDIA/modelopt-setup-codeowners` if its allowed or not." to explicitly
require review and approval: state that contributors must provide a
justification in the PR and obtain explicit approval from
`@NVIDIA/modelopt-setup-codeowners` before merging any dependency with a
non-permissive license, and replace the ambiguous phrase "wait check ... if its
allowed or not" with clear wording such as "must obtain approval from
`@NVIDIA/modelopt-setup-codeowners`."
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 42f78470-8780-4438-bf36-9711cfe49a3b
📒 Files selected for processing (3)
.coderabbit.yaml.github/PULL_REQUEST_TEMPLATE.mdCONTRIBUTING.md
05bc557 to
1e1c2a9
Compare
There was a problem hiding this comment.
♻️ Duplicate comments (1)
CONTRIBUTING.md (1)
46-46:⚠️ Potential issue | 🟠 MajorClarify and require explicit approval for non-permissive dependency licenses before merge.
Line 46 is still ambiguous (“wait check … if its allowed or not”). Please make this explicit: contributors must include PR justification and obtain review and approval from
@NVIDIA/modelopt-setup-codeownersbefore merging.Suggested wording
-If adding a new PIP dependency to any of these, make sure to verify the LICENSE of the dependency. If its not a permissive license (e.g. MIT, Apache 2), you need to provide a justification for the use of the dependency in the PR and wait check with `@NVIDIA/modelopt-setup-codeowners` if its allowed or not. +If adding a new PIP dependency to any of these, verify the dependency license first. +If it is not a permissive license (e.g., MIT, Apache 2), include justification in the PR description and obtain review and approval from `@NVIDIA/modelopt-setup-codeowners` before merge.Based on learnings: Any security-sensitive exception requires review and approval from NVIDIA/modelopt-setup-codeowners.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@CONTRIBUTING.md` at line 46, Update the dependency policy sentence that begins "If adding a new PIP dependency to any of these..." to require contributors to both add a justification for non-permissive licenses in the PR and obtain explicit review and approval from `@NVIDIA/modelopt-setup-codeowners` before merging; ensure the updated wording mentions verifying the LICENSE, lists permissive examples (MIT, Apache 2), and states that non-permissive licenses require PR justification plus documented approval from the codeowners prior to merge.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@CONTRIBUTING.md`:
- Line 46: Update the dependency policy sentence that begins "If adding a new
PIP dependency to any of these..." to require contributors to both add a
justification for non-permissive licenses in the PR and obtain explicit review
and approval from `@NVIDIA/modelopt-setup-codeowners` before merging; ensure the
updated wording mentions verifying the LICENSE, lists permissive examples (MIT,
Apache 2), and states that non-permissive licenses require PR justification plus
documented approval from the codeowners prior to merge.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: deee3412-3aff-4029-bb60-abd2b3028cde
📒 Files selected for processing (3)
.coderabbit.yaml.github/PULL_REQUEST_TEMPLATE.mdCONTRIBUTING.md
🚧 Files skipped from review as they are similar to previous changes (2)
- .github/PULL_REQUEST_TEMPLATE.md
- .coderabbit.yaml
Signed-off-by: Keval Morabia <28916987+kevalmorabia97@users.noreply.github.com>
1e1c2a9 to
4bfc05f
Compare
cuda-pythonhas mixed license and needs EStaff approval for usage. And till 0.42, it was only used inexamples/diffusers/cache_diffusion/pipelinewhich has not been updated in 9 months and not used anymore hence removing.Also cherry-picked to
release/0.42.0branch: #984Summary by CodeRabbit