Conversation
|
Caution The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased. |
📝 WalkthroughWalkthroughThe heatmap builder now restores mask source extents after resizing. The GPU Docker image updates runtime libraries, packages, cleanup, and linker configuration. The MIG20 Ray worker deployment changes to the 2.56.0-gpu image. ChangesHeatmap State Restoration
GPU Image and Deployment
Estimated code review effort: 3 (Moderate) | ~20 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 Checkov (3.3.8)docker/Dockerfile.gpuTraceback (most recent call last): helm/rayservice/workers/mig20-workers.yamlTraceback (most recent call last): 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.
Pull request overview
This PR updates the GPU deployment/image and adjusts heatmap mask resizing logic to address a top-left mask shift, while also tightening GPU Python dependency versions to avoid CUDA runtime mismatches.
Changes:
- Bump Ray worker GPU image tag in the MIG 2g.20gb worker chart.
- Pin GPU Python wheels (onnxruntime-gpu / tensorrt) in the GPU Docker image build.
- Adjust
HeatmapBuildermask resizing/cropping by temporarily switching extents to correct mask alignment.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
helm/rayservice/workers/mig20-workers.yaml |
Updates the Ray worker container image tag to a newer GPU build. |
docker/Dockerfile.gpu |
Pins CUDA-sensitive Python dependencies to reduce runtime/ABI mismatches. |
builders/heatmap_builder.py |
Changes mask resize logic (extents swap + crop) to fix mask alignment. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| # Pin CUDA 12 compatible versions to avoid libcudart.so.13 mismatches. | ||
| # The rayproject/ray:2.53.0-py312-gpu image bundles CUDA 12.x; these wheels match it. | ||
| RUN pip install --no-cache-dir \ | ||
| onnxruntime-gpu tensorrt lz4 ratiopath "mlflow<3.0" torch torchvision \ | ||
| onnxruntime-gpu==1.19.2 \ | ||
| tensorrt==10.5.0 \ |
| original_source_extents = mask_builder.source_extents.copy() | ||
| mask_builder.source_extents = mask_builder.mask_extents | ||
| try: | ||
| vips_image = mask_builder.resize_to_source(result) | ||
| finally: | ||
| mask_builder.source_extents = original_source_extents |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
docker/Dockerfile.gpu (1)
47-47: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAvoid
sudoinside Docker build steps.Run these package and linker commands as root via explicit
USERtransitions, then restore the non-root user before pip installation. This removes an unnecessary dependency onsudobeing installed and configured in the base image.Static analysis reports this pattern as DS-0010/DL3004.
Also applies to: 50-53
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docker/Dockerfile.gpu` at line 47, Remove sudo from the package cleanup commands around the Dockerfile’s systemd removal and linker-cache steps, switch to root with an explicit USER transition before those RUN instructions, then restore the existing non-root user before pip installation. Preserve the current cleanup and installation behavior while eliminating the dependency on sudo.Source: Linters/SAST tools
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@builders/heatmap_builder.py`:
- Around line 133-134: In the resize_to_source flow, update the
original_source_extents assignment to retain mask_builder.source_extents
directly instead of calling .copy(), then continue restoring that saved value
after temporarily assigning mask_builder.mask_extents.
In `@docker/Dockerfile.gpu`:
- Around line 55-64: Update the GPU image dependency installation in the
Dockerfile’s pip install block to use tested, explicit versions for torch,
torchvision, and the mlflow 3.x constraint, or install them from a generated
CUDA-specific constraints/lock file. Keep the existing CUDA-compatible pins for
onnxruntime-gpu and tensorrt, and ensure the selected Torch/TorchVision versions
are mutually compatible with the bundled CUDA 12.x runtime.
- Line 44: Update the Dockerfile.gpu dependency setup to avoid the Python 3.10
`python3-dev` package with the Python 3.12 base runtime: either remove
`python3-dev` when pip installations are wheel-only, or install matching Python
3.12 development headers and invoke pip through that environment using `python
-m pip`.
---
Nitpick comments:
In `@docker/Dockerfile.gpu`:
- Line 47: Remove sudo from the package cleanup commands around the Dockerfile’s
systemd removal and linker-cache steps, switch to root with an explicit USER
transition before those RUN instructions, then restore the existing non-root
user before pip installation. Preserve the current cleanup and installation
behavior while eliminating the dependency on sudo.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: b7d45104-13f7-4837-97a6-6563f7e0c303
📒 Files selected for processing (3)
builders/heatmap_builder.pydocker/Dockerfile.gpuhelm/rayservice/workers/mig20-workers.yaml
| original_source_extents = mask_builder.source_extents.copy() | ||
| mask_builder.source_extents = mask_builder.mask_extents |
There was a problem hiding this comment.
🩺 Stability & Availability | 🔴 Critical | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Files:"
git ls-files | sed -n '/^builders\/heatmap_builder.py$/p; /mask_builder/p; /builders.*mask/i' | sort
echo
echo "heatmap_builder outline:"
ast-grep outline builders/heatmap_builder.py || true
echo
echo "Relevant heatmap_builder lines:"
cat -n builders/heatmap_builder.py | sed -n '1,180p'
echo
echo "MaskBuilder references:"
rg -n "class MaskBuilder|def __init__|source_extents|mask_extents|\.copy\(" builders -S
echo
echo "Candidate mask files:"
fd -a 'mask.*\.py|.*builder.*\.py' . | sed 's#^\./##' | sort | rg 'builders|mask|builder' || trueRepository: RationAI/model-service
Length of output: 232
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
# Standalone behavior probe mirroring the exact concern:
# whether saving or replacing a tuple-valued attribute with .copy() fails.
class Probe:
def __init__(self):
self.s = (10, 20)
self.m = (20, 10)
probe = Probe()
print("initial source_extents:", probe.s)
try:
copy = probe.s.copy()
print("copy succeeded:", copy)
except Exception as e:
print("copy failed:", type(e).__name__, str(e))
original = probe.s
probe.s = probe.m
print("after assignment source_extents:", probe.s)
probe.s = original
print("after restore source_extents:", probe.s)
PYRepository: RationAI/model-service
Length of output: 341
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Known file:"
git ls-files | grep -F 'builders/heatmap_builder.py' || true
echo
echo "heatmap_builder lines:"
cat -n builders/heatmap_builder.py | sed -n '1,190p'
echo
echo "MaskBuilder/source_extents references:"
rg -n "class MaskBuilder|def __init__|source_extents|mask_extents|\.copy\(" builders -S || true
echo
echo "Candidate builder/mask files under builders:"
git ls-files builders | grep -E '(mask|builder).*\.py$|builders' || trueRepository: RationAI/model-service
Length of output: 7627
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Installed package availability:"
python3 - <<'PY'
try:
import ratiopath
print("ratiopath", getattr(ratiopath, "__version__", "no version"))
from ratiopath.masks.mask_builders import MaskBuilder
print("MaskBuilder:", MaskBuilder)
except Exception as e:
print(type(e).__name__, str(e))
PY
echo
echo "Search installed ratiopath mask_builders if available:"
python3 - <<'PY'
import importlib.util
spec = importlib.util.find_spec("ratiopath.masks.mask_builders")
print("spec", spec)
if spec and spec.origin:
with open(spec.origin, "r", encoding="utf-8") as f:
text = f.read()
for i, line in enumerate(text.splitlines(), 1):
if "class MaskBuilder" in line or "source_extents" in line or "mask_extents" in line or ".copy(" in line:
print(f"{spec.origin}:{i}:{line}")
PYRepository: RationAI/model-service
Length of output: 549
Avoid saving source_extents via .copy() before replacing it.
source_extents is passed to MaskBuilder as a tuple, so this line raises AttributeError each time resize_to_source() is reached and prevents the heatmap output path from finishing. Save the value directly and restore it afterward.
Proposed fix
- original_source_extents = mask_builder.source_extents.copy()
+ original_source_extents = mask_builder.source_extents📝 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.
| original_source_extents = mask_builder.source_extents.copy() | |
| mask_builder.source_extents = mask_builder.mask_extents | |
| original_source_extents = mask_builder.source_extents | |
| mask_builder.source_extents = mask_builder.mask_extents |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@builders/heatmap_builder.py` around lines 133 - 134, In the resize_to_source
flow, update the original_source_extents assignment to retain
mask_builder.source_extents directly instead of calling .copy(), then continue
restoring that saved value after temporarily assigning
mask_builder.mask_extents.
| zlib1g-dev libzstd-dev libpng-dev libjpeg-turbo8-dev libtiff-dev \ | ||
| libopenjp2-7-dev libgdk-pixbuf2.0-dev libxml2-dev sqlite3 libsqlite3-dev \ | ||
| libcairo2-dev libglib2.0-dev libdcmtk-dev libjxr-dev python3-dev | ||
| libcairo2-dev libglib2.0-dev libdcmtk-dev libjxr-dev python3-dev |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
IMAGE="${1:?usage: $0 IMAGE}"
docker run --rm "$IMAGE" bash -lc '
set -eu
python --version
python -m pip --version
python - <<'"'"'PY'
import sys
import sysconfig
from pathlib import Path
assert sys.version_info[:2] == (3, 12)
assert Path(sysconfig.get_path("include"), "Python.h").exists()
print(sys.executable)
PY
'Repository: RationAI/model-service
Length of output: 195
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
printf 'Files:\n'
git ls-files | sed -n '/docker\/Dockerfile.gpu/p;/docker.*Dockerfile.gpu/p;/Dockerfile.gpu/p'
printf '\nDockerfile.gpu relevant sections:\n'
if [ -f docker/Dockerfile.gpu ]; then
nl -ba docker/Dockerfile.gpu | sed -n '1,140p'
fi
printf '\nSearch for python3-dev usage:\n'
rg -n "python3-dev|pip install|install_requires|setup.py|pyproject|conda|uv|poetry|pip.*--no-build-isolation|--only-binary|pip wheel" docker docker/Dockerfile.gpu . --glob '!*.png' --glob '!*.jpg' --glob '!*.jpeg' --glob '!dist/**' --glob '!build/**' | sed -n '1,240p'Repository: RationAI/model-service
Length of output: 308
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
printf 'Dockerfile.gpu lines:\n'
cat -n docker/Dockerfile.gpu | sed -n '1,180p'
printf '\nTarget package and pip usage:\n'
rg -n "python3-dev|python3\.dev|pip install|pip install --only-binary|pip wheel|uv pip|pip3|/opt/conda|conda install|python3-" docker/Dockerfile.gpu . --glob '!dist/**' --glob '!build/**' || trueRepository: RationAI/model-service
Length of output: 4047
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
printf 'Ubuntu jammy python3-dev package files:\n'
apt-cache show python3-dev 2>/dev/null || true
python3 - <<'PY'
import json, urllib.request
try:
url="https://api.ubuntuarchive.org/packages/ubuntu/jammy/main/python3-dev/files"
with urllib.request.urlopen(url, timeout=20) as r:
data=json.load(r)
for f in data.get("files", [])[:80]:
print(f.get("path") or f.get("name"))
except Exception as e:
print(type(e).__name__ + ":", e)
PY
printf '\nSearch for py312-dev / python3.12-dev in repositories:\n'
python3 - <<'PY'
import urllib.request, urllib.parse, urllib.error
for query in ["python3.12-dev ubuntu jammy", "py312-dev ubuntu jammy"]:
try:
url="https://api.ubuntuarchive.org/search?q=" + urllib.parse.quote(query)
with urllib.request.urlopen(url, timeout=20) as r:
text=r.read().decode()
print(text[:500])
print("---")
except Exception as e:
print(type(e).__name__ + ":", e)
PYRepository: RationAI/model-service
Length of output: 691
🌐 Web query:
Ubuntu 22.04 jammy python3-dev package includes headers for python3.10
💡 Result:
Yes, the python3-dev package in Ubuntu 22.04 LTS (Jammy Jellyfish) includes the header files for Python 3.10 [1][2][3]. In Ubuntu 22.04, Python 3.10 is the default Python 3 version [4][5]. The python3-dev package acts as a dependency package that pulls in the development headers for the default Python 3 environment, which is python3.10-dev [1][2]. Consequently, installing python3-dev provides the necessary header files and static library required for building C extensions or other software against the system's Python 3.10 interpreter [1][6][7].
Citations:
- 1: https://packages.ubuntu.com/en/jammy/python3-dev
- 2: http://packages.ubuntu.com/jammy/python3-dev
- 3: https://packages.ubuntu.com/python3-dev
- 4: https://packages.ubuntu.com/en/jammy/python3
- 5: https://documentation.ubuntu.com/release-notes/22.04/
- 6: https://packages.ubuntu.com/jammy/python3.10-dev
- 7: https://packages.ubuntu.com/en/jammy/libpython3.10-dev
🌐 Web query:
rayproject/ray:2.53.0-py312-gpu ubuntu version python3 version
💡 Result:
The Ray Docker image tag rayproject/ray:2.53.0-py312-gpu contains the following specific components: - Python Version: This image is explicitly configured with Python 3.12, as indicated by the py312 suffix in the tag [1][2]. - Ubuntu Version: Ray official Docker images, including the 2.53.0 release series, are based on Ubuntu [1][2]. The base images for the rayproject/ray repository generally utilize ubuntu:22.04 as the underlying OS [3]. - GPU Support: The -gpu suffix indicates that the image is based on an NVIDIA CUDA image [1][4]. In the Ray project's Docker architecture, -gpu tags are aliases for specific CUDA-enabled images that require the NVIDIA Docker Runtime to access host GPUs [1][2]. In summary, the rayproject/ray:2.53.0-py312-gpu image provides a Python 3.12 environment running on an Ubuntu-based system (typically 22.04) with pre-installed CUDA support [1][2][3].
Citations:
- 1: https://github.com/ray-project/ray/blob/master/docker/ray/README.md
- 2: https://docs.ray.io/en/releases-2.53.0/ray-overview/installation.html
- 3: https://github.com/ray-project/ray/blob/master/docker/base-deps/Dockerfile
- 4: https://docs.ray.io/en/latest/ray-overview/installation.html
Use Python 3.12 headers or limit dependency installers to wheels.
The base image runs Python 3.12, but Ubuntu Jammy’s python3-dev supplies headers for the system Python 3.10. Remove python3-dev if the pip install is constrained to wheels, or install Ray/Conda Python 3.12 development headers and invoke python -m pip from that matching environment.
🧰 Tools
🪛 Trivy (0.72.0)
[error] 38-44: RUN using 'sudo'
Using 'sudo' in Dockerfile should be avoided
Rule: DS-0010
(IaC/Dockerfile)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docker/Dockerfile.gpu` at line 44, Update the Dockerfile.gpu dependency setup
to avoid the Python 3.10 `python3-dev` package with the Python 3.12 base
runtime: either remove `python3-dev` when pip installations are wheel-only, or
install matching Python 3.12 development headers and invoke pip through that
environment using `python -m pip`.
| # Pin CUDA 12 compatible versions to avoid libcudart.so.13 mismatches. | ||
| # The rayproject/ray:2.53.0-py312-gpu image bundles CUDA 12.x; these wheels match it. | ||
| RUN pip install --no-cache-dir \ | ||
| onnxruntime-gpu tensorrt lz4 ratiopath "mlflow<3.0" torch torchvision \ | ||
| onnxruntime-gpu==1.19.2 \ | ||
| tensorrt==10.5.0 \ | ||
| lz4 \ | ||
| ratiopath \ | ||
| "mlflow<3.0" \ | ||
| torch \ | ||
| torchvision \ |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
IMAGE="${1:?usage: $0 IMAGE}"
docker run --rm "$IMAGE" python - <<'PY'
import onnxruntime
import tensorrt
import torch
import torchvision
assert torch.version.cuda and torch.version.cuda.startswith("12.")
print(torch.__version__, torchvision.__version__, torch.version.cuda)
PYRepository: RationAI/model-service
Length of output: 195
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
printf 'Dockerfile.gpu context:\n'
if [ -f docker/Dockerfile.gpu ]; then
nl -ba docker/Dockerfile.gpu | sed -n '1,90p'
else
echo 'docker/Dockerfile.gpu not found'
fi
printf '\nLock/constraint references:\n'
git ls-files | rg -i '(^|/)(requirements|Pipfile|poetry|uv|conda|environment|.*lock.*|.*constraint.*|.*pin.*)\.(txt|py|toml|yml|yaml)$|(^|/)requirements|(^|/)constrain|[Ll]ock' | sed -n '1,120p'
printf '\nCUDA/Torch install commands in Dockerfiles:\n'
rg -n "torch|torchvision|tensorrt|onnxruntime-gpu|mlflow|pip install|CUDA_VERSION|NCCL|libcudart|nvidia" docker/Dockerfile.gpu -C 2 2>/dev/null || trueRepository: RationAI/model-service
Length of output: 224
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
printf 'Dockerfile.gpu context:\n'
if [ -f docker/Dockerfile.gpu ]; then
awk '{printf "%5d %s\n", NR, $0}' docker/Dockerfile.gpu | sed -n '1,90p'
else
echo 'docker/Dockerfile.gpu not found'
fi
printf '\nLock/constraint references:\n'
git ls-files | rg -i '(^|/)(requirements|Pipfile|poetry|uv|conda|environment|.*lock.*|.*constraint.*|.*pin.*)\.(txt|py|toml|yml|yaml)$|(^|/)requirements|(^|/)constrain|[Ll]ock' | sed -n '1,120p'
printf '\nCUDA/Torch install commands in Dockerfile.gpu:\n'
grep -n -E "torch|torchvision|tensorrt|onnxruntime-gpu|mlflow|pip install|CUDA_VERSION|NCCL|libcudart|nvidia" docker/Dockerfile.gpu -C 2 || trueRepository: RationAI/model-service
Length of output: 4717
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
printf 'uv.lock references for CUDA-related packages:\n'
grep -n -E 'name = "(torch|torchvision|onnxruntime-gpu|tensorrt|mlflow)"|torch|torchvision|onnxruntime|tensorrt|nvjit|cuda' uv.lock | sed -n '1,220p' || true
printf '\nOther Dockerfiles:\n'
git ls-files 'docker/Dockerfile*' 'docker/*' | sed -n '1,120p'
printf '\nDocker image references:\n'
rg -n "rayproject/ray|FROM .*gpu|torchvision|mlflow<3" . 2>/dev/null | sed -n '1,160p'Repository: RationAI/model-service
Length of output: 21547
Pin CUDA-related Python packages to the tested stack.
torch and torchvision are still unversioned in docker/Dockerfile.gpu, while uv.lock does not capture a CUDA-only lock for this image and mlflow<3.0 leaves a broad resolver range. Pin tested versions or install from a generated constraints/lock file to satisfy the stated protection against runtime/runtime and Torch/TorchVision mismatches.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docker/Dockerfile.gpu` around lines 55 - 64, Update the GPU image dependency
installation in the Dockerfile’s pip install block to use tested, explicit
versions for torch, torchvision, and the mlflow 3.x constraint, or install them
from a generated CUDA-specific constraints/lock file. Keep the existing
CUDA-compatible pins for onnxruntime-gpu and tensorrt, and ensure the selected
Torch/TorchVision versions are mutually compatible with the bundled CUDA 12.x
runtime.
fixed outdated gpu image and added fix for top left shift of mask
Summary by CodeRabbit
Bug Fixes
Chores