Skip to content

Commit 3ae6d50

Browse files
Sébastien LoiselSébastien Loisel
authored andcommitted
Remove accidental CPU↔GPU conversion helpers for type safety
- Inline GPU staging logic in repartition plans to preserve backend type - Remove _stage_cpu_to_backend and _stage_cpu_matrix_to_backend helpers - Inline _mumps_convert_to_backend (MUMPS-only conversion now explicit) - Fix map_rows to use copy(transpose(...)) preserving GPU arrays - Fix map_rows type check to use AbstractMatrix instead of Matrix - Rewrite test_map_rows.jl to use SVector as intended The type system now catches accidental CPU↔GPU mixing at dispatch time rather than relying on runtime assertions. Bump version to 0.1.4
1 parent 7e22572 commit 3ae6d50

8 files changed

Lines changed: 111 additions & 137 deletions

File tree

Project.toml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
name = "LinearAlgebraMPI"
22
uuid = "5bdd2be4-ae34-42ef-8b36-f4c85d48f377"
3-
version = "0.1.3"
3+
version = "0.1.4"
44
authors = ["S. Loisel"]
55

66
[deps]

ext/LinearAlgebraMPIMetalExt.jl

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -139,12 +139,13 @@ function LinearAlgebraMPI._array_to_backend(v::Vector{T}, ::Type{<:MtlVector}) w
139139
end
140140

141141
"""
142-
_create_output_like(v::LinearAlgebraMPI.VectorMPI{T,<:Vector}, ::Type{<:MtlVector}) where T
142+
_convert_vector_to_backend(v::LinearAlgebraMPI.VectorMPI{T,<:Vector}, ::Type{<:MtlVector}) where T
143143
144-
Create a VectorMPI with MtlVector backend from a CPU VectorMPI.
145-
Used by MUMPS factorization to reconstruct GPU output vectors.
144+
Convert a CPU VectorMPI to GPU (Metal) backend.
145+
WARNING: This function exists ONLY for MUMPS, which is a CPU-only solver.
146+
MUMPS requires GPU→CPU→GPU cycling. Do NOT use this for general operations.
146147
"""
147-
function LinearAlgebraMPI._create_output_like(v::LinearAlgebraMPI.VectorMPI{T,<:Vector}, ::Type{<:MtlVector}) where T
148+
function LinearAlgebraMPI._convert_vector_to_backend(v::LinearAlgebraMPI.VectorMPI{T,<:Vector}, ::Type{<:MtlVector}) where T
148149
return LinearAlgebraMPI.mtl(v)
149150
end
150151

src/LinearAlgebraMPI.jl

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -851,7 +851,8 @@ _matrix_to_svectors(M::AbstractMatrix{T}) where {T} = _matrix_to_svectors(Val(si
851851
# Helper: Convert Vector{SVector} back to Matrix
852852
function _svectors_to_matrix(v::AbstractVector{SVector{K,T}}) where {K,T}
853853
# reinterpret as (K, nrows), then transpose to (nrows, K)
854-
Matrix(transpose(reinterpret(reshape, T, v)))
854+
# Use copy(transpose(...)) to preserve GPU array type (not Matrix(...) which forces CPU)
855+
copy(transpose(reinterpret(reshape, T, v)))
855856
end
856857

857858
# Helper: Convert to SVector representation for map_rows
@@ -941,7 +942,7 @@ function map_rows(f, A...)
941942
row_partition = first_arg isa VectorMPI ? first_arg.partition : first_arg.row_partition
942943
hash = compute_partition_hash(row_partition)
943944

944-
if local_result isa Matrix
945+
if local_result isa AbstractMatrix
945946
return MatrixMPI(
946947
hash,
947948
row_partition,

src/dense.jl

Lines changed: 26 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -617,7 +617,8 @@ function Base.:*(A::MatrixMPI{T,AM}, x::VectorMPI{T,AV}) where {T,AM,AV}
617617
# Matrix is on CPU - compute on CPU, then copy to GPU if needed
618618
y_local_cpu = Vector{T}(undef, local_rows)
619619
LinearAlgebra.mul!(y_local_cpu, A.A, plan.gathered_cpu)
620-
y_v = _create_output_like(x.v, y_local_cpu)
620+
# Copy to GPU if input was GPU
621+
y_v = x.v isa Vector ? y_local_cpu : copyto!(similar(x.v, local_rows), y_local_cpu)
621622
else
622623
# Matrix is on GPU - use GPU gathered buffer directly
623624
y_v = similar(A.A, local_rows)
@@ -1206,8 +1207,8 @@ function Base.:*(At::Transpose{T,<:MatrixMPI{T}}, x::VectorMPI{T,AV}) where {T,A
12061207
my_col_end = A.col_partition[rank+2] - 1
12071208
local_result_cpu = full_result[my_col_start:my_col_end]
12081209

1209-
# Create output with same storage type as input
1210-
local_result = _create_output_like(x.v, local_result_cpu)
1210+
# Copy to GPU if input was GPU
1211+
local_result = x.v isa Vector ? local_result_cpu : copyto!(similar(x.v, length(local_result_cpu)), local_result_cpu)
12111212

12121213
# Create result vector (partition is immutable, no need to copy)
12131214
y = VectorMPI{T,AV}(
@@ -1470,6 +1471,11 @@ end
14701471
# DenseRepartitionPlan: Repartition a MatrixMPI to a new row partition
14711472
# ============================================================================
14721473

1474+
# Helper to get CPU copy of matrix data (no-op for CPU, copy for GPU)
1475+
# Used for MPI communication which always requires CPU buffers
1476+
_ensure_cpu(M::Matrix) = M
1477+
_ensure_cpu(M::AbstractMatrix) = Array(M)
1478+
14731479
"""
14741480
DenseRepartitionPlan{T}
14751481
@@ -1631,28 +1637,31 @@ function DenseRepartitionPlan(A::MatrixMPI{T}, p::Vector{Int}) where T
16311637
end
16321638

16331639
"""
1634-
execute_plan!(plan::DenseRepartitionPlan{T}, A::MatrixMPI{T}) where T
1640+
execute_plan!(plan::DenseRepartitionPlan{T}, A::MatrixMPI{T,AM}) where {T,AM}
16351641
16361642
Execute a dense repartition plan to redistribute rows from A to a new partition.
1637-
Returns a new MatrixMPI with the target row partition.
1643+
Returns a new MatrixMPI with the target row partition, preserving the array type (CPU/GPU).
16381644
"""
1639-
function execute_plan!(plan::DenseRepartitionPlan{T}, A::MatrixMPI{T}) where T
1645+
function execute_plan!(plan::DenseRepartitionPlan{T}, A::MatrixMPI{T,AM}) where {T,AM}
16401646
comm = MPI.COMM_WORLD
16411647

1642-
# Allocate result
1643-
result_A = Matrix{T}(undef, plan.result_local_nrows, plan.ncols)
1648+
# Get CPU version of input for MPI operations
1649+
A_cpu = _ensure_cpu(A.A)
1650+
1651+
# Allocate CPU result buffer (MPI requires CPU)
1652+
result_cpu = Matrix{T}(undef, plan.result_local_nrows, plan.ncols)
16441653

1645-
# Step 1: Local copy
1654+
# Step 1: Local copy (from CPU staging)
16461655
if !isempty(plan.local_src_range) && !isempty(plan.local_dst_range)
1647-
result_A[plan.local_dst_range, :] = A.A[plan.local_src_range, :]
1656+
result_cpu[plan.local_dst_range, :] = A_cpu[plan.local_src_range, :]
16481657
end
16491658

1650-
# Step 2: Fill send buffers and send
1659+
# Step 2: Fill send buffers and send (from CPU staging)
16511660
@inbounds for i in eachindex(plan.send_rank_ids)
16521661
r = plan.send_rank_ids[i]
16531662
row_range = plan.send_row_ranges[i]
16541663
buf = plan.send_bufs[i]
1655-
buf .= @view A.A[row_range, :]
1664+
buf .= @view A_cpu[row_range, :]
16561665
plan.send_reqs[i] = MPI.Isend(vec(buf), comm; dest=r, tag=93)
16571666
end
16581667

@@ -1667,12 +1676,15 @@ function execute_plan!(plan::DenseRepartitionPlan{T}, A::MatrixMPI{T}) where T
16671676
@inbounds for i in eachindex(plan.recv_rank_ids)
16681677
dst_range = plan.recv_dst_ranges[i]
16691678
buf = plan.recv_bufs[i]
1670-
result_A[dst_range, :] = buf
1679+
result_cpu[dst_range, :] = buf
16711680
end
16721681

16731682
MPI.Waitall(plan.send_reqs)
16741683

1675-
return MatrixMPI{T}(plan.result_structural_hash, plan.result_row_partition, plan.result_col_partition, result_A)
1684+
# Copy result back to target array type (GPU if input was GPU)
1685+
result_A = A.A isa Matrix ? result_cpu : copyto!(similar(A.A, size(result_cpu)...), result_cpu)
1686+
1687+
return MatrixMPI{T,typeof(result_A)}(plan.result_structural_hash, plan.result_row_partition, plan.result_col_partition, result_A)
16761688
end
16771689

16781690
"""

src/mumps_factorization.jl

Lines changed: 6 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -496,14 +496,12 @@ function _create_output_vector(::Type{Tin}, ::Type{AVin}, n::Int, partition) whe
496496
return _convert_vector_to_backend(v_dist, AVin)
497497
end
498498

499-
# Convert VectorMPI to a specific backend type
500-
function _convert_vector_to_backend(v::VectorMPI{T, AV}, ::Type{AVtarget}) where {T, AV, AVtarget}
501-
if AV === AVtarget
502-
return v
503-
end
504-
# Need to convert - handled by mtl/cpu functions in extension
505-
return _create_output_like(v, AVtarget)
506-
end
499+
# Convert VectorMPI to a specific backend type.
500+
# WARNING: This function exists ONLY for MUMPS, which is a CPU-only solver.
501+
# MUMPS requires GPU→CPU→GPU cycling. Do NOT use this for general operations.
502+
# The base module only defines the identity case (same type).
503+
# Extensions define CPU→GPU conversions (e.g., Vector → MtlVector).
504+
_convert_vector_to_backend(v::VectorMPI{T, AV}, ::Type{AV}) where {T, AV} = v
507505

508506
"""
509507
solve(F::MUMPSFactorizationMPI{Tin, AVin, Tinternal}, b::VectorMPI) where {Tin, AVin, Tinternal}
@@ -600,9 +598,6 @@ end
600598
# Fallback for CPU arrays
601599
_array_to_backend(v::Vector{T}, ::Type{<:Vector}) where T = v
602600

603-
# Fallback for CPU VectorMPI (no conversion needed)
604-
_create_output_like(v::VectorMPI{T,<:Vector}, ::Type{<:Vector}) where T = v
605-
606601
"""
607602
Base.:\\(F::MUMPSFactorizationMPI, b::VectorMPI)
608603

src/sparse.jl

Lines changed: 8 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1224,7 +1224,7 @@ function Base.:*(A::SparseMatrixMPI{T,Ti,AV}, B::SparseMatrixMPI{T,Ti,AV}) where
12241224
nzval = similar(A.nzval, nnz_result)
12251225

12261226
# Adapt gathered B values (plan.AT.nzval is CPU) to A's backend
1227-
AT_nzval = _create_output_like(A.nzval, plan.AT.nzval)
1227+
AT_nzval = A.nzval isa Vector ? plan.AT.nzval : copyto!(similar(A.nzval, length(plan.AT.nzval)), plan.AT.nzval)
12281228

12291229
# Execute symbolic multiplication (unified kernel)
12301230
_execute_symbolic_multiply!(nzval, plan, AT_nzval, A.nzval)
@@ -1589,11 +1589,9 @@ function Base.:+(A::SparseMatrixMPI{T,Ti,AV}, B::SparseMatrixMPI{T,Ti,AV}) where
15891589
nnz_result = plan.colptr[end] - 1
15901590
nzval = similar(A.nzval, nnz_result)
15911591

1592-
# Adapt B_repart.nzval to A's backend (repartition always returns CPU)
1593-
B_nzval = _create_output_like(A.nzval, B_repart.nzval)
1594-
15951592
# Execute addition directly into result buffer (unified kernel)
1596-
execute_addition!(nzval, plan, A.nzval, B_nzval)
1593+
# B_repart.nzval has same backend as A.nzval (repartition preserves backend)
1594+
execute_addition!(nzval, plan, A.nzval, B_repart.nzval)
15971595

15981596
# Extract CSR components directly (rowptr=colptr, colval=rowval from plan)
15991597
nrows_local = length(plan.colptr) - 1
@@ -1631,11 +1629,9 @@ function Base.:-(A::SparseMatrixMPI{T,Ti,AV}, B::SparseMatrixMPI{T,Ti,AV}) where
16311629
nnz_result = plan.colptr[end] - 1
16321630
nzval = similar(A.nzval, nnz_result)
16331631

1634-
# Adapt B_repart.nzval to A's backend (repartition always returns CPU)
1635-
B_nzval = _create_output_like(A.nzval, B_repart.nzval)
1636-
16371632
# Execute subtraction directly into result buffer (unified kernel)
1638-
execute_subtraction!(nzval, plan, A.nzval, B_nzval)
1633+
# B_repart.nzval has same backend as A.nzval (repartition preserves backend)
1634+
execute_subtraction!(nzval, plan, A.nzval, B_repart.nzval)
16391635

16401636
# Extract CSR components directly (rowptr=colptr, colval=rowval from plan)
16411637
nrows_local = length(plan.colptr) - 1
@@ -1975,7 +1971,7 @@ function execute_plan!(plan::TransposePlan{T,Ti}, A::SparseMatrixMPI{T,Ti,AV}) w
19751971
ncols_compressed = length(plan.col_indices)
19761972

19771973
# Copy result to GPU if input was GPU
1978-
result_nzval = _create_output_like(A.nzval, compressed_result_AT.nzval)
1974+
result_nzval = A.nzval isa Vector ? compressed_result_AT.nzval : copyto!(similar(A.nzval, length(compressed_result_AT.nzval)), compressed_result_AT.nzval)
19791975

19801976
# Convert structure arrays to target backend
19811977
rowptr_target = _to_target_backend(compressed_result_AT.colptr, AV)
@@ -2260,8 +2256,8 @@ function Base.:*(A::SparseMatrixMPI{T,Ti,AV}, x::VectorMPI{T,AVX}) where {T,Ti,A
22602256
# Get gathered data on CPU (MPI communication is always CPU)
22612257
gathered_cpu = _gathered_cpu_buffer(plan)
22622258

2263-
# Adapt gathered data to A's backend and allocate result
2264-
gathered = _create_output_like(A.nzval, gathered_cpu)
2259+
# Adapt gathered data to A's backend
2260+
gathered = A.nzval isa Vector ? gathered_cpu : copyto!(similar(A.nzval, length(gathered_cpu)), gathered_cpu)
22652261
y_local = similar(A.nzval, local_rows)
22662262

22672263
# Unified SpMV kernel using pre-computed target arrays (works on CPU or GPU)

src/vectors.jl

Lines changed: 21 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -111,12 +111,12 @@ function Adapt.adapt_structure(to, v::VectorMPI{T,AV}) where {T,AV}
111111
end
112112

113113
# Helper to create output vector with same storage type as reference
114-
# Used by sparse and dense A*x operations
114+
# STRICT: Only allows same-type matching. Mismatched types will error.
115+
# This prevents accidental CPU↔GPU mixing.
115116
_create_output_like(::Vector{T}, result::Vector{T}) where T = result
116-
_create_output_like(ref::AV, result::Vector{T}) where {T,AV<:AbstractVector{T}} =
117-
copyto!(similar(ref, length(result)), result)
118-
# When result is already the same type as reference, return as-is (e.g., both GPU)
119117
_create_output_like(::AV, result::AV) where {T,AV<:AbstractVector{T}} = result
118+
# Note: No catch-all method for (GPU, CPU) - this is intentional to catch bugs.
119+
# MPI staging (CPU→GPU) is inlined where needed with explicit ternary checks.
120120

121121
# Helper to get CPU copy of vector data (no-op for CPU, copy for GPU)
122122
# Used for MPI communication which always requires CPU buffers
@@ -503,31 +503,34 @@ function VectorRepartitionPlan(x::VectorMPI{T}, p::Vector{Int}) where T
503503
end
504504

505505
"""
506-
execute_plan!(plan::VectorRepartitionPlan{T}, x::VectorMPI{T}) where T
506+
execute_plan!(plan::VectorRepartitionPlan{T}, x::VectorMPI{T,AV}) where {T,AV}
507507
508508
Execute a vector repartition plan to redistribute elements from x to a new partition.
509-
Returns a new VectorMPI with the target partition.
509+
Returns a new VectorMPI with the target partition, preserving the array type (CPU/GPU).
510510
"""
511-
function execute_plan!(plan::VectorRepartitionPlan{T}, x::VectorMPI{T}) where T
511+
function execute_plan!(plan::VectorRepartitionPlan{T}, x::VectorMPI{T,AV}) where {T,AV}
512512
comm = MPI.COMM_WORLD
513513

514-
# Allocate result
515-
result_v = Vector{T}(undef, plan.result_local_size)
514+
# Get CPU version of input for MPI operations
515+
x_cpu = _ensure_cpu(x.v)
516+
517+
# Allocate CPU result buffer (MPI requires CPU)
518+
result_cpu = Vector{T}(undef, plan.result_local_size)
516519

517-
# Step 1: Local copy
520+
# Step 1: Local copy (from CPU staging)
518521
if !isempty(plan.local_src_range)
519522
@inbounds for (i, src_i) in enumerate(plan.local_src_range)
520-
result_v[plan.local_dst_offset + i - 1] = x.v[src_i]
523+
result_cpu[plan.local_dst_offset + i - 1] = x_cpu[src_i]
521524
end
522525
end
523526

524-
# Step 2: Fill send buffers and send
527+
# Step 2: Fill send buffers and send (from CPU staging)
525528
@inbounds for i in eachindex(plan.send_rank_ids)
526529
r = plan.send_rank_ids[i]
527530
range = plan.send_ranges[i]
528531
buf = plan.send_bufs[i]
529532
for (k, src_k) in enumerate(range)
530-
buf[k] = x.v[src_k]
533+
buf[k] = x_cpu[src_k]
531534
end
532535
plan.send_reqs[i] = MPI.Isend(buf, comm; dest=r, tag=92)
533536
end
@@ -544,13 +547,16 @@ function execute_plan!(plan::VectorRepartitionPlan{T}, x::VectorMPI{T}) where T
544547
offset = plan.recv_offsets[i]
545548
buf = plan.recv_bufs[i]
546549
for k in eachindex(buf)
547-
result_v[offset + k - 1] = buf[k]
550+
result_cpu[offset + k - 1] = buf[k]
548551
end
549552
end
550553

551554
MPI.Waitall(plan.send_reqs)
552555

553-
return VectorMPI{T}(plan.result_partition_hash, plan.result_partition, result_v)
556+
# Copy result back to target array type (GPU if input was GPU)
557+
result_v = x.v isa Vector ? result_cpu : copyto!(similar(x.v, length(result_cpu)), result_cpu)
558+
559+
return VectorMPI{T,typeof(result_v)}(plan.result_partition_hash, plan.result_partition, result_v)
554560
end
555561

556562
"""

0 commit comments

Comments
 (0)