-
Notifications
You must be signed in to change notification settings - Fork 80
CI/CD - Fix Image build for cuda11.1.1 and py unit test pipelines #771
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
base: main
Are you sure you want to change the base?
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #771 +/- ##
=======================================
Coverage 85.69% 85.70%
=======================================
Files 102 102
Lines 7699 7703 +4
=======================================
+ Hits 6598 6602 +4
Misses 1101 1101
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| np.percentile(result, float(percentile), interpolation='nearest'), reduce_type | ||
| ) | ||
| try: | ||
| val = np.percentile(result, float(percentile), method='nearest') |
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.
Why do we need update this? Have you met some failure?
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.
Yes, I met it after the changes for wandb. In PR, this also appeared.
'interpolation' was deprecated in v1.22.0 and removed on 2.4.0. In the python3.12 unit test pipeline, v2.4.1 was installed.
abuccts
left a 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.
pip (Python 3.8) cannot find a pre-built wheel for the latest wandb release (v0.23.1)
According to wandb v0.23.1, it's compatible with py3.8. Maybe you need to upgrade pip/wheel?
However, the build fails because the image does not have Go installed, which is required for building wandb from source.
Why not just add an apt-get install -y golang?
Added build-essential packages to the CUDA 11.1.1 Dockerfile to enable building wandb. |
| RUN add-apt-repository -y ppa:longsleep/golang-backports && \ | ||
| apt-get update && \ | ||
| apt-get install -y golang-1.24-go |
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.
merge it with existing apt-get command
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.
The 'add-apt-repository' is installed with 'software-properties-common' in the first RUN. So, we cannot combine it with the first apt-get.
| rustup update stable && \ | ||
| rustup default stable | ||
|
|
||
| ENV PATH="/usr/lib/go-1.24/bin:/root/.cargo/bin:${PATH}" |
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.
merge with existing env command
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.
The previous environment lacks Go and Cargo installations, and the subsequent environment is configured specifically for Docker workflows. Given this separation of concerns, I believe we should maintain the current configuration. Please let me know if you'd like any modifications.
| try: | ||
| val = np.percentile(result, float(percentile), method='nearest') |
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 you open another pr for this?
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.
If we move this to another PR, the python-unit-test will fail in this PR.
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 fixes CI/CD issues affecting the CUDA 11.1.1 Docker image build and Python unit test pipelines by addressing two main problems: missing build dependencies for wandb installation and deprecated NumPy API usage.
Changes:
- Added Go and Rust build tools to the CUDA 11.1.1 Dockerfile to support wandb v0.23.1 source builds
- Updated NumPy percentile API usage to support NumPy 2.x while maintaining backward compatibility with 1.x
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| superbench/benchmarks/base.py | Updated np.percentile API to use try-except for backward compatibility, preferring method='nearest' for NumPy 2.x and falling back to interpolation='nearest' for older versions |
| dockerfile/cuda11.1.1.dockerfile | Added Go 1.24, Rust, and Cargo installations with required dependencies to enable wandb source builds |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| try: | ||
| val = np.percentile(result, float(percentile), method='nearest') | ||
| except TypeError: | ||
| val = np.percentile(result, float(percentile), interpolation='nearest') |
Copilot
AI
Jan 14, 2026
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.
The try-except approach is too broad for version detection. Since TypeError could occur for other reasons, consider checking NumPy version explicitly at module level or using a more specific exception check. For example, use hasattr(np.percentile, '__wrapped__') or check np.__version__ to determine which parameter to use.
The default go version is not enough for wandb-0.23.1. |
Description
Solution