-
Couldn't load subscription status.
- Fork 183
Fixed cache diffusion ci/cd #426
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Signed-off-by: Jingyu Xin <[email protected]>
WalkthroughAdds a single dependency constraint line Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~2 minutes Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
🔇 Additional comments (1)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
examples/diffusers/cache_diffusion/requirements.txt(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: linux
- GitHub Check: code-quality
- GitHub Check: build-docs
🔇 Additional comments (1)
examples/diffusers/cache_diffusion/requirements.txt (1)
7-7: Confirm the exact transformers version.Double-check that a release tagged
4.49exists; Hugging Face typically publishes patch-qualified versions (e.g.,4.49.0). If only4.49.0is available, please pin to that exact tag.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #426 +/- ##
=======================================
Coverage 73.38% 73.38%
=======================================
Files 180 180
Lines 17934 17934
=======================================
Hits 13160 13160
Misses 4774 4774 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Signed-off-by: Keval Morabia <[email protected]>
| diffusers==0.32.2 | ||
| opencv-python>=4.8.1.78,<4.12.0.88 | ||
| peft>=0.10.0 | ||
| polygraphy==0.49.9 | ||
| sentencepiece | ||
| transformers==4.49 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we think of a better solution with correctly passing model kwargs instead of pinning to a lower version?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we only need transformers<4.57 since fix is merged in huggingface/diffusers#12438 and will be included in next 0.36 diffusers release after which we remove transformers pinned version and instead pin diffusers>=0.36.0
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, I’ve updated it. I’ll make another update once the new version is released.
Signed-off-by: Jingyu Xin <[email protected]>
|
Test passing now: https://gitlab-master.nvidia.com/omniml/modelopt/-/jobs/220234687 |
What does this PR do?
Type of change: Minor code change
Overview: ?
Usage
# Add a code snippet demonstrating how to use thisTesting
Before your PR is "Ready for review"
Additional Information
Summary by CodeRabbit