-
Notifications
You must be signed in to change notification settings - Fork 99
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
docs: Add stubs for some sparse APIs #1768
Conversation
sparse/src/KokkosSparse_par_ilut.hpp
Outdated
@@ -44,6 +44,17 @@ namespace Experimental { | |||
std::is_same<typename std::remove_const<A>::type, \ | |||
typename std::remove_const<B>::type>::value | |||
|
|||
/// @brief |
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.
Thanks @e10harvey! You are working off a fork, so I can't make changes directly to the branch, but here is what I would put for the brief:
Performs the symbolic phase of the par_ilut algorithm (described in the header). The sparsity pattern of A will be analyzed and L_rowmap and U_rowmap will be populated with the L (lower triangular) and U (upper triagular) non-zero counts respectively. Having a separate symbolic phase allows for reuse when dealing with multiple matrices with the same sparsity pattern. This routine will set some values on handle for symbolic info (row count, nnz counts).
/// @tparam KernelHandle Template for the KernelHandle type
/// @tparam ARowMapType Template for A_rowmap type
/// @tparam AEntriesType Template for A_entries type
/// @tparam LRowMapType Template for L_rowmap type
/// @tparam URowMapType Template for U_rowmap type
/// @param handle The kernel handle. It is expected that create_par_ilut_handle has been called on it
/// @param A_rowmap The row map (row nnz offsets) for the A CSR (Input)
/// @param A_entries The entries (column ids) for the A CSR (Input)
/// @param L_rowmap The row map for the L CSR, should already be sized correctly (numRows+1) (Output)
/// @param U_rowmap The row map for the U CSR, should already be sized correctly (numRows+1) (Output)
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.
@jgfouca I don't know the details of the underlying algorithm. Would it be accurate to say that the symbolic phase determines the graphs (sparsity patterns) of the
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.
@jhux2 , yes that's right. I think the idea is that there may be a use case where the user has multiple matrices with different values but the same sparsity pattern, so you can reuse the symbolic phase for all of them.
sparse/src/KokkosSparse_par_ilut.hpp
Outdated
@@ -165,6 +176,28 @@ void par_ilut_symbolic(KernelHandle* handle, ARowMapType& A_rowmap, | |||
|
|||
} // par_ilut_symbolic | |||
|
|||
/// @brief |
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.
And here's the dox for this one:
Performs the numeric phase (for specific CSRs, not reusable) of the par_ilut algorithm (described in the header). It is expected that par_ilut_symbolic has already been called for the provided KernelHandle.
/// @tparam KernelHandle Template for the handle type
/// @tparam ARowMapType Template for the A_rowmap type
/// @tparam AEntriesType Template for the A_entries type
/// @tparam AValuesType Template for the A_values type
/// @tparam LRowMapType Template for the L_rowmap type
/// @tparam LEntriesType Template for the L_entries type
/// @tparam LValuesType Template for the L_values type
/// @tparam URowMapType Template for the U_rowmap type
/// @tparam UEntriesType Template for the U_entries type
/// @tparam UValuesType Template for the U_values type
/// @param handle The kernel handle. It is expected that create_par_ilut_handle has been called on it
/// @param A_rowmap The row map (row nnz offsets) for the A CSR (Input)
/// @param A_entries The entries (column ids) for the A CSR (Input)
/// @param A_values The values (non-zero matrix values) for the A CSR (Input)
/// @param L_rowmap The row map (row nnz offsets) for the L CSR (Input/Output)
/// @param L_entries The entries (column ids) for the L CSR (Output)
/// @param L_values The values (non-zero matrix values) for the L CSR (Output)
/// @param U_rowmap The row map (row nnz offsets) for the U CSR (Input/Output)
/// @param U_entries The entries (column ids) for the U CSR (Output)
/// @param U_values The values (non-zero matrix values) for the U CSR (Output)
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.
I am leaving the deterministic arg undocumented since it is going away as soon as my latest par_ilut PR goes in.
@e10harvey , where does the doxygen for the various handle classes end up? I'm having trouble finding it on the RTD page. |
@jgfouca: The source is being parsed already but not yet included via the rst stubs we're adding here. If you add the doxygen-style comment to your handle class(es), you can tell sphinx to pull that in via this in the par_ilut section of sparse.rst:
NOTE: If you were to references |
Thanks @e10harvey , I have a brief doxy comment above the handle class and some decent doxys on the members which I think are the most important things to document. |
Co-authored-by: James Foucar <jgfouca@sandia.gov>
@jgfouca, @jhux2: Any more changes for par_ilut? @lucbv, @jennloe, @brian-kelley: I am thinking of just merging this with the par_ilut changes. You can go back and write the docs in follow-on PRs. Although, preferably this would already be done. My hope is to make things easier for everyone by adopting a process of writing and reviewing the documentation during the development process rather than having extra work in a follow-on PR / wiki edit change like this. In order for this to succeed, we have to hold each other accountable for updating the doxygen comments and .rst files during code reviews. |
@e10harvey , looks good to me! |
@e10harvey I added the Gauss-Seidel comments in a PR against your branch: e10harvey#15 |
Add doxygen for user-facing Gauss-Seidel functions
@jgfouca: Please fill in the comment content for
par_ilut
@jennloe: Please fill in the comment content for
gmres
@brian-kelley: Please fill in the comment content for
gauss_seidel
andblock_gauss_seidel
@lucbv: Please fill in the comment content for
spgemm
andblock_spgemm
Feel free to open PRs against
more_sparse_docs
or just use the suggested changes feature.Using
AT: WIP
since there's no need to run more than github-DOCS and github-FORMAT on these changes.