Skip to content

feat(inference): multi-GPU auto-layout and cross-GPU routing (PR2) - #1263

Open
greenstephen wants to merge 14 commits into
ace-step:mainfrom
greenstephen:feat/multi-gpu-pr2-auto-layout
Open

feat(inference): multi-GPU auto-layout and cross-GPU routing (PR2)#1263
greenstephen wants to merge 14 commits into
ace-step:mainfrom
greenstephen:feat/multi-gpu-pr2-auto-layout

Conversation

@greenstephen

@greenstephen greenstephen commented Jul 6, 2026

Copy link
Copy Markdown

Summary

  • Add VRAM-aware gpu_mapping=auto layout for multi-GPU CUDA systems (DiT stack on one GPU, LM on another).
  • Route conditioning tensors to the DiT GPU at inference phase boundaries (preprocess → diffusion → VAE decode).
  • Initialize the LM on device_map.lm with per-GPU vLLM memory budgeting.

Stacked on

Scope

  • In: device_map.py auto-layout, cross-GPU tensor routing, LM device wiring, tests
  • Out: CLI --gpu-mapping flag, Gradio UI panel, LM tensor parallelism (PR3–PR4)

Risk and Compatibility

  • Target path: CUDA multi-GPU inference with ACESTEP_GPU_MAPPING=auto or explicit multi-component maps
  • Non-target paths unchanged: Single-GPU behavior when mapping is unset; MPS/XPU/CPU unchanged

Regression Checks

  • acestep.test_device_map — 18 tests pass
  • acestep.core.generation.handler.init_service_test — 83/84 pass (1 pre-existing failure)

Test plan

  • ACESTEP_GPU_MAPPING=auto on 2×24GB — XL DiT + 4B LM init without OOM
  • ACESTEP_GPU_MAPPING=dit:0,vae:0,text_encoder:0,lm:1 — generate a short track
  • Single GPU, no mapping — no behavior change vs main+PR1

Made with Cursor

Summary by CodeRabbit

  • New Features
    • Added multi-component, multi-GPU device mapping with automatic VRAM-aware placement, plus a public device-mapping API for GPU discovery, layout planning, and runtime status.
    • Extended LLM startup and UI initialization to respect the mapped LM device and optional GPU mapping environment setting.
  • Bug Fixes
    • Improved device placement for conditioning inputs, intermediate tensors, silence handling, and generation timesteps to target the correct component device.
    • Fixed CUDA handling to consistently respect explicit CUDA indices during memory budgeting and vLLM/FlashAttention setup.
  • Tests
    • Expanded coverage for device-map parsing/resolution, DiT payload routing, and CUDA index–sensitive initialization behavior.

Steve and others added 2 commits July 6, 2026 12:47
Introduce per-component device mapping so DiT, VAE, text encoder, and LM
can be assigned to distinct CUDA devices while preserving single-GPU behavior
when no mapping is provided.

Co-authored-by: Cursor <cursoragent@cursor.com>
Compute VRAM-aware device placement when gpu_mapping=auto on multi-GPU
systems, route conditioning tensors to the DiT GPU at inference boundaries,
and initialize the LM on its mapped GPU with correct memory budgeting.

Co-authored-by: Cursor <cursoragent@cursor.com>
@coderabbitai

coderabbitai Bot commented Jul 6, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

This PR adds a multi-GPU device-mapping package and updates initialization, generation, LLM startup, and UI wiring to resolve and use per-component devices. It also adjusts APG guidance tensor placement to preserve input devices.

Changes

Multi-device routing and startup wiring

Layer / File(s) Summary
Device map core
acestep/device_map/*, acestep/test_device_map.py
Adds device-map primitives for parsing, discovery, auto-layout, resolution, status serialization, and tests for mapping and layout behavior.
Init service routing
acestep/core/generation/handler/init_service_*, audio_codes.py, conditioning_embed.py, generate_music_decode.py, service_generate_execute.py, gpu_config.py, llm_inference.py, init_service_test.py
Initialization, offload, generation, and LLM paths now resolve component devices, route tensors to DiT, and use CUDA-index-aware device checks and memory queries.
Startup and UI wiring
acestep/api/startup_model_init.py, startup_llm_init.py, acestep/ui/gradio/events/generation/service_init.py
Startup and UI paths now pass gpu_mapping and dit_handler, and the UI prefers the LLM device from the DiT handler map when present.
APG device placement
acestep/models/common/apg_guidance.py, acestep/models/common/apg_guidance_test.py
APG guidance returns tensors on the original input device and adds CPU, CUDA, and MPS placement coverage.

Estimated code review effort: 4 (Complex) | ~75 minutes

Sequence Diagram(s)

sequenceDiagram
  participant UI
  participant initialize_service
  participant resolve_component_device_map
  participant initialize_llm_at_startup

  UI->>initialize_service: gpu_mapping
  initialize_service->>resolve_component_device_map: requested_device, gpu_mapping
  resolve_component_device_map-->>initialize_service: ComponentDeviceMap
  initialize_service->>initialize_llm_at_startup: dit_handler
  initialize_llm_at_startup-->>UI: lm_device / startup status
Loading

Possibly related PRs

Suggested reviewers: ChuxiJ

Poem

I’m a rabbit with a GPU map,
hopping each tensor to the right lap.
DiT, VAE, and LM all get a seat,
with CUDA paths now tidy and neat.
🐇✨

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly matches the main change: multi-GPU auto-layout and cross-GPU routing for inference.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
acestep/core/generation/handler/generate_music_decode.py (1)

133-164: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Use a separate VAE device variable here (acestep/core/generation/handler/generate_music_decode.py:133-164)

vae_device is reset to None before the new VRAM check runs, so the CUDA branch can no longer use the actual VAE target device. Rename the first variable (for example, vae_target_device) and keep the decode target separate from the restore/offload device.

🤖 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 `@acestep/core/generation/handler/generate_music_decode.py` around lines 133 -
164, The VAE target device is being overwritten before the VRAM check, so the
CUDA path loses the real decode destination. Update the logic in
generate_music_decode’s VAE decode flow to keep the initial device from
_get_component_device("vae") in a separate variable (for example, a distinct
target device name) and use that for pred_latents_for_decode and the CUDA
free-memory check, while reserving the later variable for restore/offload
handling. Make sure the using_mlx_vae, vae_cpu, and
cuda_device_index/is_cuda_device branches all reference the correct device
variable so the decode target and offload target stay separate.
🧹 Nitpick comments (5)
acestep/test_device_map.py (2)

76-81: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Hardcoded env var name instead of imported constant.

"ACESTEP_GPU_MAPPING" is hardcoded here, while parse_gpu_mapping (per the upstream contract snippet) reads via a GPU_MAPPING_ENV constant. Importing and using that constant would keep the test coupled to the source of truth instead of a duplicated literal.

🤖 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 `@acestep/test_device_map.py` around lines 76 - 81, The test in
test_parse_gpu_mapping should stop hardcoding the environment variable name and
use the same source-of-truth constant as parse_gpu_mapping. Update the
patch.dict setup to reference GPU_MAPPING_ENV, so the test stays aligned with
the parser contract and avoids duplicating the literal environment key.

28-176: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Test methods lack mandatory docstrings.

None of the individual test methods in this file have docstrings (only the class docstrings are present), unlike the sibling init_service_test.py file where nearly every test method includes one.

As per coding guidelines, "Docstrings are mandatory for all new or modified Python modules, classes, and functions." Concise one-line docstrings (as used throughout init_service_test.py) would bring this file into compliance.

🤖 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 `@acestep/test_device_map.py` around lines 28 - 176, Add concise one-line
docstrings to every test method in this file so the modified tests comply with
the docstring requirement; update each method in the test classes that uses
normalize_component_device, parse_gpu_mapping, resolve_component_device_map,
compute_auto_device_map, estimate helpers, and CUDA alias helpers, following the
style used in init_service_test.py. Keep the docstrings short and descriptive,
placed directly under each test_* method definition.

Source: Coding guidelines

acestep/device_map.py (1)

54-64: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Use DeviceMapError for consistency with the rest of the module.

device_for raises bare KeyError/ValueError for invalid component names or unassigned devices, while every other validation path in this module (_format_device_for_backend, _parse_mapping_pairs, normalize_component_device, etc.) raises DeviceMapError. Downstream code that specifically catches DeviceMapError (the module's documented domain error) won't catch these.

♻️ Proposed fix
     def device_for(self, component: str) -> str:
         """Return the resolved device string for a component key."""
         key = component.strip().lower()
         if key == "model":
             key = "dit"
         if key not in _COMPONENT_KEYS:
-            raise KeyError(f"Unknown component: {component}")
+            raise DeviceMapError(f"Unknown component: {component}")
         value = getattr(self, key)
         if value is None:
-            raise ValueError(f"Component '{component}' has no assigned device")
+            raise DeviceMapError(f"Component '{component}' has no assigned device")
         return value
🤖 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 `@acestep/device_map.py` around lines 54 - 64, `device_for` should use the
module’s domain exception instead of raising bare `KeyError` and `ValueError`.
Update the validation paths in `DeviceMap.device_for` so invalid component names
and missing assignments both raise `DeviceMapError`, matching the behavior used
by `_format_device_for_backend`, `_parse_mapping_pairs`, and
`normalize_component_device` so callers can consistently catch one exception
type.
acestep/core/generation/handler/conditioning_embed.py (1)

9-16: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Docstring dependency list omits _get_component_device.

Similar to audio_codes.py, the "Depends on host members" contract doesn't list _get_component_device, now used at Lines 59-61, 70, and 133.

As per coding guidelines: "Docstrings must be concise and include purpose plus key inputs/outputs and raised exceptions when relevant."

🤖 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 `@acestep/core/generation/handler/conditioning_embed.py` around lines 9 - 16,
The docstring for ConditioningEmbedMixin has an incomplete “Depends on host
members” contract because it omits _get_component_device, which is used by the
mixin methods. Update the class docstring in ConditioningEmbedMixin to include
_get_component_device alongside the other required methods, keeping the
description concise and aligned with the existing dependency list.
acestep/core/generation/handler/audio_codes.py (1)

11-18: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Docstring dependency list omits _get_component_device.

The mixin's "Depends on host members" contract lists methods it relies on but doesn't mention _get_component_device, now used at Lines 59-60 and 87-91.

As per coding guidelines: "Docstrings must be concise and include purpose plus key inputs/outputs and raised exceptions when relevant."

🤖 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 `@acestep/core/generation/handler/audio_codes.py` around lines 11 - 18, The
AudioCodesMixin docstring’s host-dependency contract is missing the
`_get_component_device` method even though `AudioCodesMixin` uses it in its
audio parsing and latent conversion flow. Update the class docstring in
`AudioCodesMixin` so the “Depends on host members” list includes
`_get_component_device` alongside the existing required methods, keeping the
description concise and aligned with the mixin’s actual dependencies.
🤖 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 `@acestep/device_map.py`:
- Around line 1-425: This module is over the 200-LOC hard cap and should be
split by responsibility. Move the GPU/data types and small helpers like GpuInfo,
ComponentDeviceMap, DeviceMapError, and normalization/formatting utilities into
a types/parsing module, and move auto-layout logic such as LayoutRequest,
LayoutError, estimate_dit_peak_gb, estimate_lm_total_gb,
compute_auto_device_map, and discover_gpus into a layout module. Keep
resolve_component_device_map and log_device_map in a thin device_map facade that
re-exports the public API so callers of ComponentDeviceMap, parse_gpu_mapping,
and resolve_component_device_map do not need to change.
- Around line 158-203: The fallback selection in compute_auto_device_map
currently lets LM co-locate on dit_gpu based only on lm_need_gb, which can OOM
when the DiT already consumes VRAM. Update compute_auto_device_map so the LM
candidate search in the request.use_lm branch first prefers a different GPU, and
only falls back to dit_gpu if that device has enough free_vram_gb for both
estimate_dit_peak_gb(request.dit_type, request.batch_size) and
estimate_lm_total_gb(request.lm_model_path) combined; keep the
ComponentDeviceMap and LayoutError behavior unchanged otherwise.

---

Outside diff comments:
In `@acestep/core/generation/handler/generate_music_decode.py`:
- Around line 133-164: The VAE target device is being overwritten before the
VRAM check, so the CUDA path loses the real decode destination. Update the logic
in generate_music_decode’s VAE decode flow to keep the initial device from
_get_component_device("vae") in a separate variable (for example, a distinct
target device name) and use that for pred_latents_for_decode and the CUDA
free-memory check, while reserving the later variable for restore/offload
handling. Make sure the using_mlx_vae, vae_cpu, and
cuda_device_index/is_cuda_device branches all reference the correct device
variable so the decode target and offload target stay separate.

---

Nitpick comments:
In `@acestep/core/generation/handler/audio_codes.py`:
- Around line 11-18: The AudioCodesMixin docstring’s host-dependency contract is
missing the `_get_component_device` method even though `AudioCodesMixin` uses it
in its audio parsing and latent conversion flow. Update the class docstring in
`AudioCodesMixin` so the “Depends on host members” list includes
`_get_component_device` alongside the existing required methods, keeping the
description concise and aligned with the mixin’s actual dependencies.

In `@acestep/core/generation/handler/conditioning_embed.py`:
- Around line 9-16: The docstring for ConditioningEmbedMixin has an incomplete
“Depends on host members” contract because it omits _get_component_device, which
is used by the mixin methods. Update the class docstring in
ConditioningEmbedMixin to include _get_component_device alongside the other
required methods, keeping the description concise and aligned with the existing
dependency list.

In `@acestep/device_map.py`:
- Around line 54-64: `device_for` should use the module’s domain exception
instead of raising bare `KeyError` and `ValueError`. Update the validation paths
in `DeviceMap.device_for` so invalid component names and missing assignments
both raise `DeviceMapError`, matching the behavior used by
`_format_device_for_backend`, `_parse_mapping_pairs`, and
`normalize_component_device` so callers can consistently catch one exception
type.

In `@acestep/test_device_map.py`:
- Around line 76-81: The test in test_parse_gpu_mapping should stop hardcoding
the environment variable name and use the same source-of-truth constant as
parse_gpu_mapping. Update the patch.dict setup to reference GPU_MAPPING_ENV, so
the test stays aligned with the parser contract and avoids duplicating the
literal environment key.
- Around line 28-176: Add concise one-line docstrings to every test method in
this file so the modified tests comply with the docstring requirement; update
each method in the test classes that uses normalize_component_device,
parse_gpu_mapping, resolve_component_device_map, compute_auto_device_map,
estimate helpers, and CUDA alias helpers, following the style used in
init_service_test.py. Keep the docstrings short and descriptive, placed directly
under each test_* method definition.
🪄 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: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: d997cfaf-8d50-442e-92fa-345fa8fee757

📥 Commits

Reviewing files that changed from the base of the PR and between 6d467e4 and a38527f.

📒 Files selected for processing (17)
  • acestep/api/startup_llm_init.py
  • acestep/api/startup_model_init.py
  • acestep/core/generation/handler/audio_codes.py
  • acestep/core/generation/handler/conditioning_embed.py
  • acestep/core/generation/handler/generate_music_decode.py
  • acestep/core/generation/handler/init_service_catalog.py
  • acestep/core/generation/handler/init_service_loader.py
  • acestep/core/generation/handler/init_service_offload_context.py
  • acestep/core/generation/handler/init_service_orchestrator.py
  • acestep/core/generation/handler/init_service_setup.py
  • acestep/core/generation/handler/init_service_test.py
  • acestep/core/generation/handler/service_generate_execute.py
  • acestep/device_map.py
  • acestep/gpu_config.py
  • acestep/llm_inference.py
  • acestep/test_device_map.py
  • acestep/ui/gradio/events/generation/service_init.py

Comment thread acestep/device_map.py Outdated
Comment thread acestep/device_map.py Outdated
Steve and others added 2 commits July 6, 2026 15:07
Move GPU discovery, layout, parsing, resolution, and status helpers into
focused submodules under acestep/device_map/ with a stable public facade.

Co-authored-by: Cursor <cursoragent@cursor.com>
…ndex.

Route generate_kwargs tensors to the DiT device, move latents during
preprocess, and preserve CUDA device indices in APG project().

Co-authored-by: Cursor <cursoragent@cursor.com>

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

🧹 Nitpick comments (2)
acestep/device_map/discovery.py (1)

21-24: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Narrow the exception type flagged by Ruff (BLE001).

except Exception is a blind catch-all; a RuntimeError (or narrower torch-specific exception) would satisfy the "catch specific exceptions" guideline while preserving the safe None fallback.

♻️ Proposed narrowing
         try:
             capability = torch.cuda.get_device_capability(index)
-        except Exception:
+        except RuntimeError:
             capability = None
🤖 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 `@acestep/device_map/discovery.py` around lines 21 - 24, The exception handling
in the device capability lookup is too broad and triggers Ruff BLE001. In the
function that calls torch.cuda.get_device_capability, replace the blanket except
Exception with a narrower exception type such as RuntimeError or the most
specific torch-related exception that can be raised here, while keeping the
existing fallback of setting capability to None.

Source: Linters/SAST tools

acestep/models/common/apg_guidance_test.py (1)

12-59: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Add coverage for the MPS device-restoration branch.

Tests cover CPU and non-default CUDA-index device preservation, but not the device.type == "mps" branch in project() — the exact path with the device-restoration bug flagged in apg_guidance.py. Since real MPS hardware likely isn't available in CI, consider mocking torch.device.type/monkeypatching to exercise that branch and assert the returned tensors land back on the original (mocked) MPS device.

🤖 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 `@acestep/models/common/apg_guidance_test.py` around lines 12 - 59, The
device-preservation tests in ApgGuidanceDeviceTests currently miss the
mps-specific restoration path inside project(), which is the branch tied to the
reported bug. Add a test that exercises the device.type == "mps" logic by
mocking or monkeypatching the device check in project() so the branch runs
without real MPS hardware. Verify that both returned tensors from project() are
restored to the original mocked MPS device, similar to the existing CPU and
cuda:1 assertions.
🤖 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 `@acestep/core/generation/handler/init_service_memory_basic.py`:
- Around line 154-163: The `_ensure_silence_latent_on_device` method is using
`_is_on_target_device`, which only checks device type and can miss GPU index
mismatches when `device_map.dit` points to a different CUDA device. Update the
device validation at this call site to compare the full normalized target device
for `silence_latent` against the DiT device (or `self.device` when no
`device_map` is set), and only move it when the exact device index does not
match.

In `@acestep/device_map/layout.py`:
- Around line 79-93: The LM placement logic in request.use_lm can still choose
dit_gpu even when its remaining VRAM after DiT allocation is insufficient.
Update the lm_gpu selection in layout.py to account for the DiT reservation when
evaluating colocated placement, using the LM budget helper or an equivalent
remaining-capacity check with reserve_dit_inference_gb set appropriately. Keep
the fix localized around estimate_lm_total_gb, lm_candidates, and the lm_gpu
selection so the fallback to dit_gpu only succeeds when the post-DiT headroom is
actually enough.

In `@acestep/models/common/apg_guidance.py`:
- Around line 21-34: The tensor projection helper in apg_guidance currently
overwrites the original MPS device after the CPU fallback, so the return tensors
stay on CPU instead of being moved back. Update the device handling in the
projection function that computes v0_parallel and v0_orthogonal so it preserves
the original device before any .cpu() fallback, then uses that saved device in
the final .to(...) calls to return tensors to MPS correctly.

---

Nitpick comments:
In `@acestep/device_map/discovery.py`:
- Around line 21-24: The exception handling in the device capability lookup is
too broad and triggers Ruff BLE001. In the function that calls
torch.cuda.get_device_capability, replace the blanket except Exception with a
narrower exception type such as RuntimeError or the most specific torch-related
exception that can be raised here, while keeping the existing fallback of
setting capability to None.

In `@acestep/models/common/apg_guidance_test.py`:
- Around line 12-59: The device-preservation tests in ApgGuidanceDeviceTests
currently miss the mps-specific restoration path inside project(), which is the
branch tied to the reported bug. Add a test that exercises the device.type ==
"mps" logic by mocking or monkeypatching the device check in project() so the
branch runs without real MPS hardware. Verify that both returned tensors from
project() are restored to the original mocked MPS device, similar to the
existing CPU and cuda:1 assertions.
🪄 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: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 6b980c49-a248-448c-b2c6-b125501c0592

📥 Commits

Reviewing files that changed from the base of the PR and between a38527f and 7d2b302.

📒 Files selected for processing (18)
  • acestep/core/generation/handler/conditioning_embed.py
  • acestep/core/generation/handler/init_service_memory_basic.py
  • acestep/core/generation/handler/init_service_setup.py
  • acestep/core/generation/handler/init_service_test.py
  • acestep/core/generation/handler/service_generate_execute.py
  • acestep/device_map/__init__.py
  • acestep/device_map/constants.py
  • acestep/device_map/devices.py
  • acestep/device_map/discovery.py
  • acestep/device_map/errors.py
  • acestep/device_map/layout.py
  • acestep/device_map/parsing.py
  • acestep/device_map/resolve.py
  • acestep/device_map/status.py
  • acestep/device_map/types.py
  • acestep/models/common/apg_guidance.py
  • acestep/models/common/apg_guidance_test.py
  • acestep/test_device_map.py
✅ Files skipped from review due to trivial changes (2)
  • acestep/device_map/errors.py
  • acestep/device_map/init.py
🚧 Files skipped from review as they are similar to previous changes (4)
  • acestep/core/generation/handler/service_generate_execute.py
  • acestep/core/generation/handler/conditioning_embed.py
  • acestep/core/generation/handler/init_service_setup.py
  • acestep/test_device_map.py

Comment thread acestep/core/generation/handler/init_service_memory_basic.py
Comment thread acestep/device_map/layout.py
Comment thread acestep/models/common/apg_guidance.py
Steve and others added 3 commits July 6, 2026 17:09
…ack.

Also narrow get_device_capability exception handling to RuntimeError.

Co-authored-by: Cursor <cursoragent@cursor.com>
Reject unsupported default_device backends explicitly in GPU mapping
parsing, and preserve the original MPS device when project() uses CPU
math fallback.

Co-authored-by: Cursor <cursoragent@cursor.com>
_is_on_target_device only compares backend types, so cuda:0 and cuda:3
look equivalent. Use exact device matching when placing silence_latent on
the DiT device from device_map.

Co-authored-by: Cursor <cursoragent@cursor.com>

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
acestep/core/generation/handler/init_service_memory_basic.py (1)

114-125: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick win

Narrow the blind except Exception.

Ruff flags this as BLE001. normalize_component_device only raises DeviceMapError (on empty string), and torch.device(...) raises RuntimeError/TypeError on invalid strings — catching those specifically avoids silently swallowing unrelated bugs.

As per coding guidelines, "Error handling: Avoid bare except: clauses; catch specific exceptions."

♻️ Proposed fix
+from acestep.device_map.errors import DeviceMapError
+
     def _tensor_on_exact_device(self, tensor, target_device: str) -> bool:
         """Return whether *tensor* is on the exact device string (including CUDA index)."""
         if tensor is None:
             return True
         from acestep.device_map.devices import normalize_component_device

         try:
             expected = torch.device(normalize_component_device(str(target_device)))
-        except Exception:
+        except (DeviceMapError, RuntimeError, TypeError):
             return False
         return tensor.device == expected
🤖 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 `@acestep/core/generation/handler/init_service_memory_basic.py` around lines
114 - 125, The `_tensor_on_exact_device` helper in
`init_service_memory_basic.py` is catching `Exception` too broadly, which
triggers BLE001. Update the `try`/`except` around `normalize_component_device`
and `torch.device(...)` to catch only the expected failures from those calls,
using the specific exception types they can raise, and leave other errors
unhandled so real bugs are not hidden.

Sources: Coding guidelines, 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.

Nitpick comments:
In `@acestep/core/generation/handler/init_service_memory_basic.py`:
- Around line 114-125: The `_tensor_on_exact_device` helper in
`init_service_memory_basic.py` is catching `Exception` too broadly, which
triggers BLE001. Update the `try`/`except` around `normalize_component_device`
and `torch.device(...)` to catch only the expected failures from those calls,
using the specific exception types they can raise, and leave other errors
unhandled so real bugs are not hidden.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: b5b2f4bf-cedd-4eac-bc44-25b891fab657

📥 Commits

Reviewing files that changed from the base of the PR and between c1d9d98 and b6d4061.

📒 Files selected for processing (2)
  • acestep/core/generation/handler/init_service_memory_basic.py
  • acestep/core/generation/handler/init_service_test.py

Steve and others added 6 commits July 6, 2026 17:36
Module-level import avoids per-call overhead; catch DeviceMapError and
RuntimeError instead of a broad Exception in _tensor_on_exact_device.

Co-authored-by: Cursor <cursoragent@cursor.com>
Replace bare Exception handlers with DeviceMapError/RuntimeError/TypeError
for exact device checks and RuntimeError/TypeError/ValueError for backend
alias parsing in _is_on_target_device.

Co-authored-by: Cursor <cursoragent@cursor.com>
vae_component_device was overwritten with None before the CUDA free-VRAM
check, so multi-GPU layouts always queried cuda:0. Keep a separate
vae_restore_device for the CPU offload restore path.

Co-authored-by: Cursor <cursoragent@cursor.com>
Wrap cuda_device_index int() parsing so strings like cuda:x raise the
module's domain error instead of a raw ValueError.

Co-authored-by: Cursor <cursoragent@cursor.com>
Move shared fixtures into generate_music_decode_test_support.py and split
scenario tests into prepare, basic, CPU-offload, and VRAM modules.

Co-authored-by: Cursor <cursoragent@cursor.com>
Drop the custom sys.modules package stubbing loader; import the mixin
through the public package path like other handler tests.

Co-authored-by: Cursor <cursoragent@cursor.com>
@pokepress

Copy link
Copy Markdown

So, just to put some comments specific to this PR here, the code in llm_inference.py only respects the LM device setting if the device setting is "cuda" as opposed to "auto". Additionally, it seems like the backend needs to be set to PT since VLLM doesn't seem to be respecting the setting and uses the same GPU for all models. Let me know if you have other things you want me to try.

@greenstephen

Copy link
Copy Markdown
Author

Thanks — good catches, and useful to keep them on this PR.

auto vs cuda: Agreed. In llm_inference.initialize, device == "auto" is collapsed to bare "cuda" (no index), so a mapped cuda:1 never sticks on that path. Related: several later checks use device == "cuda" / device != "cuda", which also break indexed devices. One concrete footgun is the vLLM gate:

if backend == "vllm" and device != "cuda":
# falls back to pt
So "cuda:1" is treated as “not CUDA for vLLM” and gets forced to PT, while bare "cuda" stays on vLLM and lands on the default GPU (usually 0).

vLLM same-GPU behavior: That matches what we’re seeing. Until indexed CUDA is handled cleanly in the vLLM init path, --backend pt is the reliable way to keep the LM on lm:N. We’ll treat proper vLLM + cuda:N as part of the same fix.

Things worth trying:

--backend pt with --gpu-mapping "dit:0,vae:0,text_encoder:0,lm:1" and --init-service true
Confirm logs show Loading LLM to cuda:1 (not bare cuda)
Watch nvidia-smi during LM vs DiT phases
We’ll fix the Gradio/device_map wiring plus the bare-"cuda" comparisons in a follow-up on this stack. Appreciate the testing.

Resolve Gradio lm_device after DiT initialize_service so device_map.lm
is applied, normalize/preserve indexed CUDA devices in llm_inference,
and move text-encoder token ids onto the text-encoder component device.

Co-authored-by: Cursor <cursoragent@cursor.com>
@greenstephen

Copy link
Copy Markdown
Author

@pokepress Thanks again — you were right about the LM landing on bare cuda.

I’ve pushed a fix on this branch (ec02094) that addresses:

  1. Gradio init orderinglm_device is now taken from device_map after DiT initialize_service, so lm:1 isn’t lost on first UI init when the device dropdown is auto.
  2. llm_inference CUDA indexingauto / bare cuda normalize to cuda:0; mapped cuda:N is preserved. The vLLM gate now uses is_cuda_device() so cuda:1 is no longer treated as “not CUDA” and forced to PT.
  3. Related — text-encoder token ids are moved onto the text-encoder device before encode (helps split maps).

Please retry (after pulling this branch):

uv run acestep \
  --init-service true \
  --config-path acestep-v15-xl-sft \
  --lm_model_path acestep-5Hz-lm-4B \
  --init-llm true \
  --gpu-mapping "dit:0,vae:0,text_encoder:0,lm:1"

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.

3 participants