Skip to content

Conversation

@ParticularMiner
Copy link
Contributor

  • This PR fixes "first" and "last" aggregators so that they can also support dask_grblas.
  • It also restores the specification: my_matrix.diag(k, ...) (extraction of a diagonal from a Matrix to a Vector) which helps enable the support for dask-grblas. This specification was removed earlier in PR added X.diag(k, ...) where type(X) in {Vector, Matrix} #186 .

Feel free to edit this PR if anything is amiss!

@jim22k jim22k merged commit 022ecfe into python-graphblas:main Mar 24, 2022
@eriknw
Copy link
Member

eriknw commented Mar 24, 2022

Hooray, thanks @ParticularMiner 🎉

@eriknw eriknw mentioned this pull request Mar 24, 2022
index = step1[0].new().value
if index is None:
# `==` instead of `is` automatically triggers index.compute() in dask-grblas:
if index == None: # noqa
Copy link
Member

Choose a reason for hiding this comment

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

We might want to consider supporting e.g. t = v[s].new() where s and t are both gb.Scalars that are empty. Currently, scalar indices are GrB_Index type, not GrB_Scalar type, so this isn't supported by SuiteSparse:GraphBLAS. But, we could support this in grblas and dask-grblas, and doing so would allow us to avoid dask-grblas computing during index == None.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Great idea from an optimization standpoint!

Heh, this automatically means also dealing with lists of indices of different types, like v[2 s 1 None t].new().

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants