Skip to content

Avoid double-counting CPU RAM for integrated CUDA - #4186

Open
arpansahu wants to merge 1 commit into
huggingface:mainfrom
arpansahu:fix-integrated-cuda-max-memory
Open

Avoid double-counting CPU RAM for integrated CUDA#4186
arpansahu wants to merge 1 commit into
huggingface:mainfrom
arpansahu:fix-integrated-cuda-max-memory

Conversation

@arpansahu

Copy link
Copy Markdown

Problem

get_max_memory() adds a separate "cpu" memory entry even when CUDA reports an integrated GPU. Integrated CUDA devices share host RAM, so downstream device-map planning can treat one physical memory pool as independent CUDA and CPU capacities. Closes #4183.

Reproducer

from types import SimpleNamespace
from unittest.mock import patch
import torch
from accelerate.utils import modeling
from accelerate.utils.modeling import get_max_memory

with patch.object(modeling, "is_mps_available", return_value=False), \
     patch.object(torch.cuda, "device_count", return_value=1), \
     patch.object(torch.cuda, "mem_get_info", return_value=(1234, 5678)), \
     patch.object(torch.cuda, "get_device_properties", return_value=SimpleNamespace(is_integrated=True)), \
     patch.object(torch, "tensor"):
    print(get_max_memory())
# before: {0: 1234, "cpu": ...}
# after:  {0: 1234}

Fix

Track whether any available CUDA device reports is_integrated and skip adding a separate CPU memory budget in that case. The attribute is read with getattr(..., False) so older PyTorch/device properties keep the existing behavior.

Testing

Added tests/test_modeling_utils.py::ModelingUtilsTester::test_get_max_memory_integrated_cuda_does_not_add_cpu.

Before the fix:

FAILED tests/test_modeling_utils.py::ModelingUtilsTester::test_get_max_memory_integrated_cuda_does_not_add_cpu
1 failed in 7.37s

After the fix:

2 passed, 1 skipped in 6.56s
7 passed, 6 skipped in 69.63s (0:01:09)

Quality:

ruff check ... --fix: All checks passed!
ruff format ...: 2 files left unchanged
ruff check ...: All checks passed!
ruff format --check ...: 2 files already formatted

Integrated CUDA devices share host memory, so exposing both the device budget and a separate CPU budget lets automatic device mapping treat one physical pool as two. Skip the CPU entry when CUDA device properties report an integrated GPU.\n\nCo-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Signed-off-by: arpan sahu <28574248+arpansahu@users.noreply.github.com>
@arpansahu

Copy link
Copy Markdown
Author

Disclosure: this contribution was prepared with AI assistance (GitHub Copilot CLI), and
I am flagging that explicitly rather than leaving you to guess.

The defect, the fix and the regression test were verified locally: the new test fails on
unmodified upstream and passes with the change applied, and the surrounding suite is green.

If your project would prefer not to take AI-assisted contributions, or you would rather this
were reworked and resubmitted by hand, please just close it -- no objection at all, and
apologies for the noise.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

get_max_memory() adds a separate cpu entry on integrated CUDA GPUs, double-counting shared RAM (GB10 / DGX Spark)

1 participant