Skip to content

Add adapt definitions for SparseArrays#91

Merged
maleadt merged 2 commits intoJuliaGPU:masterfrom
mtfishman:adapt_sparsearrays
Feb 19, 2025
Merged

Add adapt definitions for SparseArrays#91
maleadt merged 2 commits intoJuliaGPU:masterfrom
mtfishman:adapt_sparsearrays

Conversation

@mtfishman
Copy link
Contributor

@mtfishman mtfishman commented Feb 13, 2025

Closes #90.

With this PR the new behavior is:

julia> using Adapt, SparseArrays

julia> a = sprandn(2, 2, 0.5)
2×2 SparseMatrixCSC{Float64, Int64} with 2 stored entries:
 2.27897  -0.00209377
            

julia> adapt(Array, a)
2×2 SparseMatrixCSC{Float64, Int64} with 2 stored entries:
 2.27897  -0.00209377
            

julia> adapt(Array{Float32}, a)
2×2 SparseMatrixCSC{Float32, Int64} with 2 stored entries:
 2.27897  -0.00209377
            

@github-actions
Copy link

github-actions bot commented Feb 13, 2025

Your PR requires formatting changes to meet the project's style guidelines.
Please consider running Runic (git runic master) to apply these changes.

Click here to view the suggested changes.
diff --git a/src/Adapt.jl b/src/Adapt.jl
index fe5f817..9b79755 100644
--- a/src/Adapt.jl
+++ b/src/Adapt.jl
@@ -73,12 +73,12 @@ end
 
 @static if !isdefined(Base, :get_extension)
 function __init__()
-    @require SparseArrays = "2f01184e-e22b-5df5-ae63-d93ebab69eaf" begin
-        include("../ext/AdaptSparseArraysExt.jl")
-    end
-    @require StaticArrays = "90137ffa-7385-5640-81b9-e52037218182" begin
-        include("../ext/AdaptStaticArraysExt.jl")
-    end
+        @require SparseArrays = "2f01184e-e22b-5df5-ae63-d93ebab69eaf" begin
+            include("../ext/AdaptSparseArraysExt.jl")
+        end
+        @require StaticArrays = "90137ffa-7385-5640-81b9-e52037218182" begin
+            include("../ext/AdaptStaticArraysExt.jl")
+        end
 end
 end
 

@maleadt
Copy link
Member

maleadt commented Feb 18, 2025

julia> adapt(Array, a)
2×2 SparseMatrixCSC{Float64, Int64} with 2 stored entries:
 2.27897  -0.00209377
            

I don't get this; you're asking for things to become an Array, yet you're disabling the materialization of a sparse array, which isn't an Array? For most intents and purposes, adapt should behave like convert, with the exception that it peeks through structural layers (structs, tuples, etc). convert(Array, ::SparseMatrixCSC) most definitely returns an Array, so I don't see why adapt(Array, ::SparseMatrixCSC) shouldn't.

@codecov
Copy link

codecov bot commented Feb 19, 2025

Codecov Report

Attention: Patch coverage is 75.00000% with 2 lines in your changes missing coverage. Please review.

Project coverage is 91.56%. Comparing base (447e7b8) to head (f6cb009).
Report is 4 commits behind head on master.

Files with missing lines Patch % Lines
src/Adapt.jl 50.00% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master      #91      +/-   ##
==========================================
- Coverage   93.42%   91.56%   -1.86%     
==========================================
  Files           6        7       +1     
  Lines          76       83       +7     
==========================================
+ Hits           71       76       +5     
- Misses          5        7       +2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@maleadt
Copy link
Member

maleadt commented Feb 19, 2025

nightly failures happen on master too.

Copy link
Member

@maleadt maleadt left a comment

Choose a reason for hiding this comment

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

@maleadt maleadt merged commit c80a6ac into JuliaGPU:master Feb 19, 2025
19 of 22 checks passed
@mtfishman mtfishman deleted the adapt_sparsearrays branch February 19, 2025 14:46
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.

Should adapt(::Type{<:Array}, ::SparseMatrixCSC) preserve sparsity?

2 participants