Skip to content
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

[CUSOLVER] Add ordering functions #1198

Merged
merged 8 commits into from
Oct 19, 2021
Merged

Conversation

amontoison
Copy link
Member

No description provided.

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.

Thanks! Couple of comments (that apply to all of these functions).

rowsA = A.rowPtr |> Vector{Cint}
colsA = A.colVal |> Vector{Cint}
p = zeros(Cint, n)
cusolverSpXcsrsymrcmHost(sparse_handle(), n, nnz(A), descA, rowsA, colsA, p)
Copy link
Member

Choose a reason for hiding this comment

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

Why do you use the host APIs here?

Copy link
Member Author

Choose a reason for hiding this comment

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

  • symrcm, symmdq, symamd and metisnd are used inside cusolverRF (Add a wrapper to cusolverRF in CUSOLVER #828) but no low-level wrappers are implemented.
  • zfd should be combined in practice with ilu0 routine (incomplete LU factorization) because the factorization is done without pivoting and if one diagonal element is zero the factorization failed.

Copy link
Member

Choose a reason for hiding this comment

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

Ah, there aren't any device equivalents for these APIs...

n, m = A.dims
(m ≠ m) && throw(DimensionMismatch("A must be square, but has dimensions ($n,$m)!"))
descA = CUSPARSE.CuMatrixDescriptor('G', 'L', 'N', index)
rowsA = A.rowPtr |> Vector{Cint}
Copy link
Member

Choose a reason for hiding this comment

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

We don't want to do this.

lib/cusolver/CUSOLVER.jl Outdated Show resolved Hide resolved
lib/cusolver/ordering.jl Outdated Show resolved Hide resolved
lib/cusolver/ordering.jl Outdated Show resolved Hide resolved
lib/cusolver/ordering.jl Outdated Show resolved Hide resolved
test/cusolver/ordering.jl Outdated Show resolved Hide resolved
@amontoison
Copy link
Member Author

@maleadt I take into account all your comments.

@codecov
Copy link

codecov bot commented Oct 19, 2021

Codecov Report

Merging #1198 (8923705) into master (211cfb1) will increase coverage by 79.96%.
The diff coverage is 100.00%.

Impacted file tree graph

@@             Coverage Diff             @@
##           master    #1198       +/-   ##
===========================================
+ Coverage    0.49%   80.45%   +79.96%     
===========================================
  Files         118      119        +1     
  Lines        7953     8374      +421     
===========================================
+ Hits           39     6737     +6698     
+ Misses       7914     1637     -6277     
Impacted Files Coverage Δ
lib/cusolver/sparse.jl 92.25% <100.00%> (+92.25%) ⬆️
lib/utils/enum.jl 75.00% <0.00%> (ø)
lib/cudadrv/CUDAdrv.jl 48.83% <0.00%> (+4.65%) ⬆️
examples/wmma/high-level.jl 11.11% <0.00%> (+11.11%) ⬆️
examples/wmma/low-level.jl 14.28% <0.00%> (+14.28%) ⬆️
examples/hello_world.jl 16.66% <0.00%> (+16.66%) ⬆️
lib/cudnn/tensor.jl 18.96% <0.00%> (+18.96%) ⬆️
lib/cupti/error.jl 20.00% <0.00%> (+20.00%) ⬆️
lib/library_types.jl 20.00% <0.00%> (+20.00%) ⬆️
src/sorting.jl 22.26% <0.00%> (+22.26%) ⬆️
... and 109 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 211cfb1...8923705. Read the comment docs.

test/cusolver/sparse.jl Outdated Show resolved Hide resolved
@maleadt
Copy link
Member

maleadt commented Oct 19, 2021

Thanks, looks good!

@maleadt maleadt merged commit 1c6752c into JuliaGPU:master Oct 19, 2021
xaellison pushed a commit to xaellison/CUDA.jl that referenced this pull request Oct 20, 2021
simonbyrne pushed a commit to simonbyrne/CUDA.jl that referenced this pull request Nov 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cuda libraries Stuff about CUDA library wrappers. enhancement New feature or request needs changes Changes are needed.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants