AbstractGBMatrix Rewrite #64
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Codecov Report
@@ Coverage Diff @@
## master #64 +/- ##
==========================================
- Coverage 35.94% 35.23% -0.71%
==========================================
Files 49 50 +1
Lines 3703 3675 -28
==========================================
- Hits 1331 1295 -36
- Misses 2372 2380 +8
Continue to review full report at Codecov.
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Pretty major rewrite, solves several issues:
A GBMatrix now stores
Ref{Ptr{GB_Matrix_opaque}}rather thanPtr{GB_Matrix_opaque}. This allows us to avoid complexity with managing multiple GBMatrices that "own" the same C level GrB_Matrix.Most functions accept an AbstractGBMatrix/Vector/Array. This makes it much easier to create a new subtype that overloads some subset of the functionality.
A GBMatrix (and an AbstractGBArray) now has a
fillfield. This defaults tonothingto replicate the current behavior. But for true matrices (not graphs)zeromakes more sense.For all current datatypes
fillonly interacts with indexing. It does not participate inmap,mul,reduce,ewise,selectetc by virtue of 1. being a Julia level construct, and 2. GraphBLAS attaches fill values to the operators, not to the storage.All of the above should make it easier to implement GBMatrixCSC (although it unfortunately won't live under
AbstractSparseMatrixCSC), alias things correctly, and interop far better with Julia AbstractArray interface.