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
ENH: Update MLIR backend to LLVM 20.dev #799
ENH: Update MLIR backend to LLVM 20.dev #799
Changes from all commits
d511c5c
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.
This should probably handle SOA and without SOA separately.
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.
In my opinion we should only support SoA singleton format:
What would be the benefit of supporting non-SoA singleton levels separately?
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.
We'd be able to support the current COO format only for non-SoA.
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.
What do you mean by that? Can you give some example?
With this PR we can support COO format (also input objects of type
scipy.sparse.coo_array
) where MLIR-backend implementation uses SoA representation.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 existing Numba
COO
format uses the non-SoA format, and we pretty much have to support this to be backwards compatible. Doesn't have to be in this PR though.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 think I'm still missing the point here. Numba is a separate backend that supports only 1D and 2D COO arrays. MLIR backend supports >=2D COO arrays.
What do you mean by backward compatibility here? Can you give an example where backward compatibility breaks here? If a user passes
scipy.sparse.coo_array
object tosparse.asarray
function it it will work for any backend regardless of an internal representation (SoA or non-SoA).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.
So,
sparse.COO
has a constructor that takescoords
and a.coords
attribute. The attribute is a 2D NumPy array.Ideally, I'd like to keep it a 2D
np.ndarray
as otherwise I'm not 100% sure how much will break.We could do this with
np.stack
, but that would incur a performance penalty.Also the current Numba backend supports ND, not 2D. SciPy supports only 2D, however.
I'm thinking of a future where all of sparse is powered by Finch-MLIR, ideally.
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 think for accessing
coords
,np.stack
could be an option. My caveat for non-SoA in MLIR backend is that it already has some issues with basic operations: https://discourse.llvm.org/t/passmanager-fails-on-simple-coo-addition-example/81247