Skip to content

Conversation

eriknw
Copy link
Member

@eriknw eriknw commented Apr 25, 2022

I think the tests could be improved. Some of the NetworkX tests get coverage,
but only compare to 0 triangles. Also, we should test more with self-edges.

There may be better ways to compute triangles for:

  • all nodes
  • a subset of nodes
  • a single node

There are a lot of different ways to compute triangles, so this could be
explored further in the future. I hope the current PR is competitive.

I think the tests could be improved. Some of the NetworkX tests get coverage, but only compare to 0 triangles. Also, we should test more with self-edges. There may be better ways to compute triangles for: - all nodes - a subset of nodes - a single node There are *a lot* of different ways to compute triangles, so this could be explored further in the future. I hope the current PR is competitive.
@eriknw eriknw requested a review from jim22k April 25, 2022 05:23
def single_triangle_core(G, index):
M = Matrix(bool, G.nrows, G.ncols)
M[index, index] = False
C = any_pair(G.T @ M.T).new(name="C") # select.coleq(G, index)
Copy link
Member Author

Choose a reason for hiding this comment

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

Check it out, @jim22k, a use case for coleq select op!

Hmm, the comment is wrong. Should be select.coleq(G.T, index) or select.col(G.T == index).

@eriknw
Copy link
Member Author

eriknw commented Apr 25, 2022

Triangle counting raises the inevitable question: how should we handle properties (such as has_self_edges) and cached computation such as lower and upper triangles? I say "inevitable", because we encountered the same issues in metagraph and LAGraph.

For now, they are passed as arguments to functions and computed if necessary.

Comment on lines +31 to +36
C = plus_pair(L @ L.T).new(mask=L.S)
return (
C.reduce_rowwise().new(mask=mask)
+ C.reduce_columnwise().new(mask=mask)
+ plus_pair(U @ L.T).new(mask=U.S).reduce_rowwise().new(mask=mask)
).new(name="triangles")
Copy link
Member Author

Choose a reason for hiding this comment

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

I wish I had an intuitive explanation for why this worked (I'm sure one exists if you twist your brain just so). There are simpler methods, but this implementation seems pretty fast.

Here's an example of a simpler method:

 L = gb.select.tril(G, -1).new(name="L") B = gb.select.offdiag(G).new(name="offdiag") return plus_pair(B @ L.T).new(mask=B.S).reduce_rowwise().new(mask=mask)
@eriknw eriknw merged commit e30c0b2 into python-graphblas:main Apr 27, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

1 participant