Skip to content
Open
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
8 changes: 5 additions & 3 deletions src/interface.jl
Original file line number Diff line number Diff line change
Expand Up @@ -198,10 +198,12 @@ function coloring(
A::AbstractMatrix,
problem::ColoringProblem,
algo::GreedyColoringAlgorithm;
decompression_eltype::Type{R}=Float64,
decompression_eltype::Type=Float64,

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The rationale behind the type annotation was to force specialization: https://docs.julialang.org/en/v1/manual/performance-tips/#Be-aware-of-when-Julia-avoids-specializing

Not sure how this change makes anything better in that regard, I would have thought it makes things worse?

@amontoison amontoison Sep 7, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Me too and it is the contrary that is happening, the compiler widen the type to DataType internally from what I unserstand if we keep Type{R} and we don't have the inference in 3 cases.

I am wondering if it is because the argument with the specialization is a keyword argument.

symmetric_pattern::Bool=false,
) where {R}
return _coloring(WithResult(), A, problem, algo, R, symmetric_pattern)
)
return _coloring(
WithResult(), A, problem, algo, decompression_eltype, symmetric_pattern
)
end

"""
Expand Down
39 changes: 39 additions & 0 deletions test/type_stability.jl
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,15 @@ using Test

rng = StableRNG(63)

# `@inferred` and `@test_opt` build their keyword arguments with `typeof`, which erases
# `Type{Float64}` down to `DataType`. Passing `decompression_eltype` straight to them would
# therefore hide the eltype from inference and make every result type unresolvable. Positional
# arguments go through `Core.Typeof` instead, which keeps `Type{R}` intact, so this wrapper is
# what lets us check that `coloring` stays inferrable when the eltype is given explicitly.
function coloring_with_eltype(A, problem, algo, ::Type{R}) where {R}
return coloring(A, problem, algo; decompression_eltype=R)
end

@testset "Sparse coloring" begin
n = 10
A = sparse(Symmetric(sprand(rng, n, n, 5 / n)))
Expand Down Expand Up @@ -57,6 +66,36 @@ rng = StableRNG(63)
)
end
end

@testset "Explicit decompression_eltype" begin
@testset "$structure - $partition - $decompression - $R" for (
structure, partition, decompression
) in [
(:nonsymmetric, :column, :direct),
(:nonsymmetric, :row, :direct),
(:symmetric, :column, :direct),
(:symmetric, :column, :substitution),
(:nonsymmetric, :bidirectional, :direct),
(:nonsymmetric, :bidirectional, :substitution),
],
R in (Float64, Float32)

@testset for order in all_orders()
@test_opt coloring_with_eltype(
A,
ColoringProblem(; structure, partition),
GreedyColoringAlgorithm(order; decompression),
R,
)
@inferred coloring_with_eltype(
A,
ColoringProblem(; structure, partition),
GreedyColoringAlgorithm(order; decompression),
R,
)
end
end
end
end;

@testset "Structured coloring" begin
Expand Down
Loading