Skip to content
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

Add pagerank example #673

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

Add pagerank example #673

wants to merge 4 commits into from

Conversation

mtsokol
Copy link
Collaborator

@mtsokol mtsokol commented May 7, 2024

Hi @hameerabbasi,

Here I share initial pagerank implementation (following scipy_pagerank).

A few comments:

  • It's slower than scipy implementation.
  • spdiag is still missing in finch-tensor, therefore I use SciPy diag to create it.

@mtsokol mtsokol self-assigned this May 7, 2024
Copy link

github-actions bot commented May 7, 2024

Test Results

5 923 tests  ±0   5 891 ✅ ±0   9m 24s ⏱️ -7s
    1 suites ±0      32 💤 ±0 
    1 files   ±0       0 ❌ ±0 

Results for commit b2e63af. ± Comparison against base commit c12b29e.

This pull request skips 2 and un-skips 2 tests.
sparse.numba_backend.tests.test_compressed ‑ test_reductions_float16[f4-None-sum-kwargs0]
sparse.numba_backend.tests.test_coo ‑ test_reductions_float16[f8-None-sum-kwargs0]
sparse.numba_backend.tests.test_compressed ‑ test_reductions_float16[i4-None-sum-kwargs0]
sparse.numba_backend.tests.test_coo ‑ test_reductions_float16[i4-None-sum-kwargs0]

♻️ This comment has been updated with latest results.

Comment on lines +25 to +26
Q = sparse.asarray(sp.csc_array(sp.spdiags(S.todense(), 0, *A.shape)))
A = Q @ A
Copy link
Collaborator

Choose a reason for hiding this comment

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

IIUC This is equivalent to https://data-apis.org/array-api/latest/extensions/generated/array_api.linalg.diagonal.html#diagonal, which is supported by the Numba backend. Let's use that instead.

Suggested change
Q = sparse.asarray(sp.csc_array(sp.spdiags(S.todense(), 0, *A.shape)))
A = Q @ A
Q = sparse.diagonal(S)
A *= Q

Copy link
Collaborator

Choose a reason for hiding this comment

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

For Finch, we can add support for it later, as the example only needs to work on Numba for now.

Copy link
Collaborator

Choose a reason for hiding this comment

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

You might need Q[None, :] to match the results, but otherwise this is equivalent.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

But the pagerank example is run either with Finch or Numba backend. when we run with Finch backend (and I think it should be runnable with Finch) then we can't execute one line with Numba.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I would prefer to keep these examples Finch and Numba ready, and update implementation once both backends support something new.

Copy link
Collaborator

@hameerabbasi hameerabbasi May 10, 2024

Choose a reason for hiding this comment

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

I would prefer to keep these examples Finch and Numba ready, and update implementation once both backends support something new.

According to the deliverable, we just need a script that will demonstrate a speedup against the "old" code. If you would prefer, we can always keep this PR open until xp.diagonal is supported by Finch, but I wouldn't densify.

@mtsokol mtsokol force-pushed the pagerank-example branch from 0217874 to 4dba84e Compare May 9, 2024 10:55
@mtsokol mtsokol force-pushed the pagerank-example branch from 4dba84e to b2e63af Compare May 14, 2024 13:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants