-
Notifications
You must be signed in to change notification settings - Fork 1.4k
resolves CI's drive-out-of-space by prune caching and use torch fro CPU #8673
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
📝 WalkthroughWalkthroughReplaces custom multi-step pip caching in the GitHub Actions workflow with the built-in Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Cache: Disabled due to data retention organization setting Knowledge base: Disabled due to 📒 Files selected for processing (1)
🧰 Additional context used📓 Path-based instructions (1)**/*.py⚙️ CodeRabbit configuration file
Files:
⏰ 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). (19)
🔇 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.
Pull request overview
This PR optimizes CI workflow resource usage by simplifying pip caching and ensuring consistent use of PyTorch CPU-only wheels to resolve drive space issues.
- Replaced custom cache management with built-in
actions/setup-pythoncaching - Added
--extra-index-url https://download.pytorch.org/whl/cputo pip install commands for smaller PyTorch installations
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
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: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/workflows/pythonapp.yml (1)
86-86: Missing CPU wheel index flag.Line 86 installs PyTorch without
--extra-index-url https://download.pytorch.org/whl/cpu, causing CUDA wheels (~2GB+) to be downloaded instead of CPU wheels, defeating the PR's disk space reduction objective.🔎 Proposed fix
- python -m pip install torch==2.5.1 torchvision==0.20.1 + python -m pip install torch==2.5.1 torchvision==0.20.1 --extra-index-url https://download.pytorch.org/whl/cpu
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge base: Disabled due to Reviews -> Disable Knowledge Base setting
📒 Files selected for processing (1)
.github/workflows/pythonapp.yml
⏰ 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). (20)
- GitHub Check: Agent
- GitHub Check: flake8-py3 (pytype)
- GitHub Check: flake8-py3 (mypy)
- GitHub Check: flake8-py3 (codeformat)
- GitHub Check: quick-py3 (macOS-latest)
- GitHub Check: quick-py3 (ubuntu-latest)
- GitHub Check: min-dep-py3 (3.10)
- GitHub Check: build-docs
- GitHub Check: quick-py3 (windows-latest)
- GitHub Check: min-dep-os (windows-latest)
- GitHub Check: packaging
- GitHub Check: min-dep-pytorch (2.5.1)
- GitHub Check: min-dep-py3 (3.11)
- GitHub Check: min-dep-pytorch (2.8.0)
- GitHub Check: min-dep-py3 (3.12)
- GitHub Check: min-dep-pytorch (2.7.1)
- GitHub Check: min-dep-py3 (3.9)
- GitHub Check: min-dep-pytorch (2.6.0)
- GitHub Check: min-dep-os (macOS-latest)
- GitHub Check: min-dep-os (ubuntu-latest)
🔇 Additional comments (3)
.github/workflows/pythonapp.yml (3)
36-36: Built-in pip caching correctly applied.This simplifies cache management as intended.
69-69: LGTM: Consistent pip caching across all jobs.Built-in caching mechanism correctly applied.
Also applies to: 119-119, 190-190
127-127: CPU wheel index correctly applied.All pip install commands properly configured to use CPU PyTorch wheels.
Also applies to: 156-156, 167-167, 177-177, 194-194
|
Hi @Borda thanks for this fix, I think this is good to work as the tests have passed here without the space issue occurring. Please do follow the instructions for the DCO issue and have a look at anything Coderabbit said, even the ones outside the diff range. For the mypy problem, I think some library somewhere else we haven't pinned closely was updated and so the typing is broken, you may need to put |
In fact, it complains about the fix, but it is not correct as GHA uses a pure CPU machine, and so installing the CUDA/GPU version doesn't have any impact, only takes extra space which in this case is the root of the problem...
Yes, I planned to tackle the mypy in a separate PR to keep the changes focused and minimal to be easier reviewed, but sure I can add it here :) |
…f `torch` Signed-off-by: jirka <jirka.borovec@seznam.cz>
Signed-off-by: jirka <jirka.borovec@seznam.cz>
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.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Signed-off-by: jirka <jirka.borovec@seznam.cz>
|
/build |
@KumoLiu passed 🟢 🎉 |
This pull request simplifies the caching setup for pip dependencies in the GitHub Actions workflow and ensures that all pip installs use the PyTorch CPU wheel index. The main changes involve replacing custom cache steps with the built-in
cache: 'pip'option inactions/setup-python, and adding the--extra-index-urlflag to all relevant pip install commands for consistent dependency resolution.Workflow caching improvements:
actions/cacheand manual timestamp keys) with the built-incache: 'pip'option inactions/setup-python, streamlining cache management across all jobs. [1] [2] [3] [4]Dependency installation updates:
pip installcommands to include--extra-index-url https://download.pytorch.org/whl/cpu, ensuring that PyTorch and related packages are consistently installed from the CPU wheel index. This affects installation from wheels, tarballs, requirements files, and direct package installs. [1] [2] [3] [4] [5]