-
Notifications
You must be signed in to change notification settings - Fork 17
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Support for SparseMatrixCSC{Float64, Int32}
#144
Comments
Unfortunately that’s not possible. The underlying library only supports 64
bit indices.
…On Sat, Sep 21, 2024 at 10:36 AM Royi ***@***.***> wrote:
See the code below:
mT = SparseMatrixCSC{Float64, int32}(sprand(1_000, 1_000, 0.01));GBMatrix(mT); #<! Error!
The last line will just error.
It would be great to have support for SparseMatrixCSC{T, N} where T <:
Union{Float16, Float32, Float64, Float128}, N <: Union{Int16, Int32,
Int64}.
—
Reply to this email directly, view it on GitHub
<#144>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/ACG6QV5JJQIF5CO3AGJRULLZXV77TAVCNFSM6AAAAABOTUUN6SVHI2DSMVQWIX3LMV43ASLTON2WKOZSGU2DAMZSGU3DCMY>
.
You are receiving this because you are subscribed to this thread.Message
ID: ***@***.***>
|
Support does not mean change the underlying. Even something like that: GBMatrix(SparseMatrixCSC{elttype(mT), Int64}(mT)); Will do. |
Ah okay sure I can do that.
…On Sat, Sep 21, 2024 at 1:44 PM Royi ***@***.***> wrote:
Support does not mean change the underlying.
Just support converting it without creating an error.
Even something like that:
GBMatrix(SparseMatrixCSC{elttype(mT), Int64}(mT));
Will do.
—
Reply to this email directly, view it on GitHub
<#144 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ACG6QVYV3VGPHZ2HUJKKZQLZXWV7LAVCNFSM6AAAAABOTUUN6SVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDGNRVGI3DKOJQGU>
.
You are receiving this because you commented.Message ID:
***@***.***>
|
Is there a real reason not to support Just wonder if there any point in asking for it. |
I’ve asked and talked with Tim extensively about it.
It’s maybe more possible now than it used to be, but it would be a very
significant change. Fixing 32 bit indexing in SPQR alone was a ~5k line
diff. This would be closer to ~40k.
And there’s no way to do it in a standards compliant way. We haven’t added
32 bit indices to the standard, although I could push for it. It ends up
being much more valuable for pattern matrices or Boolean than for Float64.
So I can see the value add.
…On Sat, Sep 21, 2024 at 1:51 PM Royi ***@***.***> wrote:
Is there a real reason not to support Int32 in the c code?
Does it mean a lot of work?
Just wonder if there any point in asking for it.
—
Reply to this email directly, view it on GitHub
<#144 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ACG6QV6YQ3GH2DCOEW5STCDZXWW3HAVCNFSM6AAAAABOTUUN6SVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDGNRVGI3DOOBVGI>
.
You are receiving this because you commented.Message ID:
***@***.***>
|
The added value is speed. I don't know about GraphBLAS, but for certain in many sparse related algorithms it makes a difference since most of the data to process are indices. So the cache is twice as big effectively, the bandwidth, etc... |
As I mentioned it's only valuable when the data type of your values is small, and it's not twice always twice as fast. It depends on the internal representation. I agree with you, it's very valuable especially as people often deal with smaller types these days. I'll bring it up in committee meetings but I wouldn't expect progress on that front until next year at the earliest. |
See the code below:
The last line will just error.
It would be great to have support for
SparseMatrixCSC{T, N}
whereT <: Union{Float16, Float32, Float64, Float128}
,N <: Union{Int16, Int32, Int64}
.The text was updated successfully, but these errors were encountered: