Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
79 changes: 79 additions & 0 deletions tests/diffusion/lora/test_loader.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
WanLoraLoaderMixin,
_prepare_lora_delta,
_remap_state_dict_keys,
_resolve_lora_compute_device,
)

HEAD_DIM = 24
Expand Down Expand Up @@ -521,3 +522,81 @@ def test_prepare_lora_delta_with_compute_device():
assert delta is not None
assert delta.shape == (HEAD_DIM, HEAD_DIM)
assert len(used_keys) == 2


def test_resolve_lora_compute_device_probes_unusable_accelerator(mocker: MockerFixture):
"""cuda:N/npu:N may be reported without a physical device; the probe must reject it."""
mocker.patch(
"vllm_omni.diffusion.lora.loader.get_local_device",
return_value=torch.device("cuda:999"),
)
assert _resolve_lora_compute_device() is None


def test_load_lora_into_module_cpu_weights_match_cpu_reference(mocker: MockerFixture):
"""Regression test: params on CPU used to force compute_device=None and skip the
#7005 accelerator path. With an unusable accelerator, fusion must fall back to
CPU and still produce base + lora_B @ lora_A."""
pipeline = DummyPipeline(NUM_LAYERS, HEAD_DIM)
original_params = {name: param.clone() for name, param in pipeline.transformer.named_parameters()}

lora_state_dict = make_lora_state_dict_for_module(pipeline.transformer)
lora_state_dict = _remap_state_dict_keys(lora_state_dict, [(".to_out.0.", ".to_out.")])

mocker.patch(
"vllm_omni.diffusion.lora.loader.get_local_device",
return_value=torch.device("cuda:999"),

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This patches get_local_device to cuda:999, so _resolve_lora_compute_device() returns None and load_lora_into_module never takes the #7005 accelerator path. The assertion only checks CPU base + B @ A after fallback — the same result as the pre-fix compute_dev = None behavior. Add a second test that patches _resolve_lora_compute_device to a non-CPU device, spies _prepare_lora_delta, asserts compute_device is that device for CPU params, and sets compute_device=None inside the spy so the numerical check can stay on CPU.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Addressed: Added test_load_lora_into_module_forwards_usable_accel_device :

  • Patches _resolve_lora_compute_device to return a non-CPU device, bypassing the probe to simulate a usable accelerator.
  • Spies _prepare_lora_delta and asserts every CPU param receives that device as compute_device — this fails if the assignment regresses back to else None.
  • The spy forces compute_device=None for the actual math, so the numerical check still runs on CPU without a physical device.
    21 tests pass on a no-GPU/NPU container.

)

used_keys = pipeline.load_lora_into_module(lora_state_dict, pipeline.transformer)
assert len(used_keys) > 0
assert not (set(lora_state_dict.keys()) - used_keys)

for name, param in pipeline.transformer.named_parameters():
if not name.endswith(".weight"):
continue
base_key = name[: -len(".weight")]
key_a = f"{base_key}.lora_A.weight"
key_b = f"{base_key}.lora_B.weight"
if key_a in lora_state_dict and key_b in lora_state_dict:
expected = original_params[name] + (
lora_state_dict[key_b].to(param.dtype) @ lora_state_dict[key_a].to(param.dtype)
)
assert_close(param, expected)


def test_load_lora_into_module_forwards_usable_accel_device(mocker: MockerFixture):
"""A usable accelerator must be forwarded as compute_device for CPU params.
A spy forces the actual math back onto CPU so the test runs without a device."""
fake_dev = torch.device("cuda:0")
mocker.patch(
"vllm_omni.diffusion.lora.loader._resolve_lora_compute_device",
return_value=fake_dev,
)

seen_devices = []
original = _prepare_lora_delta

def spy(state_dict, base_key, *args, **kwargs):
seen_devices.append(kwargs.get("compute_device"))
kwargs["compute_device"] = None
return original(state_dict, base_key, *args, **kwargs)

mocker.patch(
"vllm_omni.diffusion.lora.loader._prepare_lora_delta",
side_effect=spy,
)

pipeline = DummyPipeline(NUM_LAYERS, HEAD_DIM)
original_params = {name: param.clone() for name, param in pipeline.transformer.named_parameters()}

lora_state_dict = make_lora_state_dict_for_module(pipeline.transformer)
lora_state_dict = _remap_state_dict_keys(lora_state_dict, [(".to_out.0.", ".to_out.")])

used_keys = pipeline.load_lora_into_module(lora_state_dict, pipeline.transformer)

# Every CPU param with a LoRA delta must have received the accelerator device.
assert seen_devices
assert all(d == fake_dev for d in seen_devices)
assert not (set(lora_state_dict.keys()) - used_keys)
assert_params_not_equal(pipeline.transformer, original_params)
27 changes: 25 additions & 2 deletions vllm_omni/diffusion/lora/loader.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
from safetensors.torch import load_file
from vllm.logger import init_logger

from vllm_omni.diffusion.distributed.utils import get_local_device
from vllm_omni.diffusion.utils.tf_utils import get_transformer_from_pipeline
from vllm_omni.transformers_utils.repo_utils import hf_api

Expand Down Expand Up @@ -120,6 +121,22 @@ def _prepare_lora_delta(
return delta, used_keys


def _resolve_lora_compute_device() -> torch.device | None:
"""Return the local accelerator device if actually usable, else None.

get_local_device() may return cuda:N/npu:N even when no physical device is
present, so probe with a tiny allocation before trusting the lookup.
"""
try:
dev = get_local_device()
if dev.type != "cpu":
torch.empty(1, device=dev)
return dev
except Exception:
pass
return None


def _load_lora_state_dict(
pretrained_model_name_or_path: str,
weights_name: str | None = None,
Expand Down Expand Up @@ -270,6 +287,9 @@ def load_lora_into_module(
for param_name, weight_name, _ in module.stacked_params_mapping:
param_to_weight_names[param_name].append(weight_name)

# Resolve the accelerator fallback once, outside the parameter loop.
accel_dev = _resolve_lora_compute_device()

for name, params in module.named_parameters(prefix):
is_bias = False
if name.endswith(".bias"):
Expand All @@ -280,7 +300,7 @@ def load_lora_into_module(
else:
continue

compute_dev = params.device if params.device.type != "cpu" else None
compute_dev = params.device if params.device.type != "cpu" else accel_dev

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This is the line that actually activates NPU/GPU matmul for CPU weights, and nothing in the new tests observes it. A regression that restores else None would still pass test_load_lora_into_module_cpu_weights_match_cpu_reference. Cover this assignment (usable accel_dev forwarded as compute_device) rather than only the unusable-device fallback.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Addressed: Added test_load_lora_into_module_forwards_usable_accel_device :
Patches _resolve_lora_compute_device to return a non-CPU device, bypassing the probe to simulate a usable accelerator.
Spies _prepare_lora_delta and asserts every CPU param receives that device as compute_device — this fails if the assignment regresses back to else None.
The spy forces compute_device=None for the actual math, so the numerical check still runs on CPU without a physical device.
21 tests pass on a no-GPU/NPU container.

delta, used_keys = _prepare_lora_delta(
state_dict,

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

No regression test covers the bug this PR describes: params.device.type == "cpu" used to force compute_device=None and skip the #7005 path. Existing test_prepare_lora_delta_with_compute_device only passes torch.device("cpu") into _prepare_lora_delta directly. Add a load_lora_into_module test on a CPU module that patches get_local_device to a fake accelerator device and asserts the fused weight equals a CPU lora_B @ lora_A reference (and still works when the helper returns CPU / is unavailable).

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Addressed: test_load_lora_into_module_cpu_weights_match_cpu_reference: patches get_local_device to a fake accelerator (cuda:999) on a CPU module and asserts fused weights equal the CPU reference base + lora_B @ lora_A. All 20 tests pass on a no-GPU/NPU container.

base_key,
Expand Down Expand Up @@ -324,6 +344,9 @@ def unload_module_lora(
for param_name, weight_name, _ in module.stacked_params_mapping:
param_to_weight_names[param_name].append(weight_name)

# Resolve the accelerator fallback once, outside the parameter loop.
accel_dev = _resolve_lora_compute_device()

for name, param in module.named_parameters(prefix):
is_bias = False
if name.endswith(".bias"):
Expand All @@ -334,7 +357,7 @@ def unload_module_lora(
else:
continue

compute_dev = param.device if param.device.type != "cpu" else None
compute_dev = param.device if param.device.type != "cpu" else accel_dev
delta, used_keys = _prepare_lora_delta(
state_dict,
base_key,
Expand Down
Loading