-
Notifications
You must be signed in to change notification settings - Fork 8
Accept AbstractSparseMatrixCSC for decompression #298
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
base: main
Are you sure you want to change the base?
Changes from all commits
22b37fd
bd06fd3
c351e79
b8451af
d4a736a
6ad9a2f
a029f83
e93a786
9469e4d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -46,6 +46,7 @@ compress | |
| decompress | ||
| decompress! | ||
| decompress_single_color! | ||
| decompress_csc! | ||
| ``` | ||
|
|
||
| ## Orders | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -585,8 +585,23 @@ function decompress!( | |
| return A | ||
| end | ||
|
|
||
| function decompress!( | ||
| A::SparseMatrixCSC{R}, | ||
| """ | ||
| decompress_csc!( | ||
| nzA::AbstractVector{R}, | ||
| A_colptr::AbstractVector, | ||
| B::AbstractMatrix{R}, | ||
| result::TreeSetColoringResult, | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It seems a bit weird to only add this for the acyclic case, you don't need it for anything else on the JuMP side?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. True, I haven't tested with star coloring yet. That's one of the advantage of using SMC, now we can easily test with star coloring as well :) |
||
| uplo::Symbol=:F, | ||
| ) where {R<:Real} | ||
|
|
||
| Decompress the values of `B` into the vector of nonzero entries `nzA` of a | ||
| sparse matrix with column pointers `colptr`. This function assumes that the | ||
| row indices are sorted in increasing order and are the same as those of the | ||
| sparse matrix given to the `coloring` function that returned `result`. | ||
| """ | ||
| function decompress_csc!( | ||
| nzA::AbstractVector{R}, | ||
| A_colptr::AbstractVector{<:Integer}, | ||
| B::AbstractMatrix{R}, | ||
| result::TreeSetColoringResult, | ||
| uplo::Symbol=:F, | ||
|
|
@@ -603,10 +618,6 @@ function decompress!( | |
| upper_triangle_offsets, | ||
| buffer, | ||
| ) = result | ||
| (; S) = ag | ||
| A_colptr = A.colptr | ||
| nzA = nonzeros(A) | ||
| check_compatible_pattern(A, ag, uplo) | ||
|
|
||
| if eltype(buffer) == R | ||
| buffer_right_type = buffer | ||
|
|
@@ -696,6 +707,17 @@ function decompress!( | |
| #! format: on | ||
| end | ||
| end | ||
| return nothing | ||
| end | ||
|
|
||
| function decompress!( | ||
| A::SparseMatrixCSC{R}, | ||
| B::AbstractMatrix{R}, | ||
| result::TreeSetColoringResult, | ||
| uplo::Symbol=:F, | ||
| ) where {R<:Real} | ||
| check_compatible_pattern(A, result.ag, uplo) | ||
| decompress_csc!(nonzeros(A), A.colptr, B, result, uplo) | ||
| return A | ||
| end | ||
|
|
||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The specific result types we use are not public right now, so this function cannot be public either
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It was failing if not included in the docs since it has a docstring.
What do you mean by result types not being public ?
decompress!also takesresultas argumentThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I mean that the precise names and fields of the results which are returned are subject to change.
That's also why I asked whether we wanted to extend
decompress_csc!beyond just the acyclic coloring case, because otherwise we have to document that it only works for a specific type of coloring. In any case, I'd rather keep it in the private part of the package and list the docstring among the internals, so that you can start using it but we're free to refine the interface in future versions.