Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
[FEA] Add support for bitmap_view & the API of
bitmap_to_csr
#2109[FEA] Add support for bitmap_view & the API of
bitmap_to_csr
#2109Changes from 7 commits
165b278
62d9e34
7dea38d
1b6e784
67f0650
1e5294c
70cce47
f8960d9
20f76af
7dc7cf8
cab8691
3edeafd
f015ed5
308eb8b
0d5ec74
5cf5a8b
42d5355
1aff844
59533e0
311f2d1
7d960bd
8a3b759
7dea754
08397c6
b2ad06e
09e64ab
53202c6
8838a36
1c91c74
01dd903
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
Our sparse APIs discern between
strucure owning
andstructure preserving
. The former means that the nnz is not yet known when the user creates the object. The latter means that nnz is known upon creation and any processing functions do not have the freedom to change it. One of the reasons we discern between these two is so that upon invoking a function, the user knows whether the function is supposed to compute the sparsity in addition to populating the resulting non-zero weights/elements. It doesn't look like we make that distinction here, yet we are calculating and computing the sparsity in the implementation.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.
Does it mean we should calculate/get the
nnz
of bitmap and reallocate/allocate values by it forcsr
in the API?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 elements of the csr_view should be initialized to
1
at the end of this conversion function. Also check that thecsr_view.nnz
correspond to the NNZ that was computed to avoid memory issues while writing incsr_view.indices
.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 agree with you on checking 'nnz', but it can hurt the performance; I intended to recommend the caller to check
nnz
when building thebitset
before calling. Do you think this is okay?