test: working on benchmarks with `Finch` as backend by DeaMariaLeon · Pull Request #772 · pydata/sparse

DeaMariaLeon · 2024-09-10T14:27:56Z

What type of PR is this? (check all applicable)

Related issues

Related issue #
Closes #

Checklist

Code follows style guide
Tests added
Documented the changes

Please explain your changes below.

Only added Finch on config.py - (locally I see the names of the backend. But I'm not sure if this renders fine on CodSpeed).
Also, per Hameer instructions I need to see which tests fail with Finch.
I would like to see if the change on codspeed.yml works like expected.
Finally, I have only added the line if hasattr(sparse, "compiled"): f = sparse.compiled(f) to the first two benchmarks to make sure I correctly understood.

for more information, see https://pre-commit.ci

codspeed-hq · 2024-09-10T15:39:49Z

CodSpeed Performance Report

Merging #772 will not alter performance

_{Comparing DeaMariaLeon:bench2 (f822c24) with main (47b246b)}

Summary

✅ 27 untouched benchmarks

🆕 313 new benchmarks
⁉️ 313 dropped benchmarks

⚠️ Please fix the performance issues or acknowledge them on CodSpeed.

Benchmarks breakdown

	Benchmark	`main`	`DeaMariaLeon:bench2`	Change
🆕	`test_elemwise[backend='Numba'-side=100-rank=1-format='coo'-add]`	N/A	3.5 ms	N/A
🆕	`test_elemwise[backend='Numba'-side=100-rank=1-format='coo'-mul]`	N/A	2.7 ms	N/A
🆕	`test_elemwise[backend='Numba'-side=100-rank=1-format='gcxs'-add]`	N/A	4.1 ms	N/A
🆕	`test_elemwise[backend='Numba'-side=100-rank=1-format='gcxs'-mul]`	N/A	3.3 ms	N/A
🆕	`test_elemwise[backend='Numba'-side=100-rank=2-format='coo'-add]`	N/A	4 ms	N/A
🆕	`test_elemwise[backend='Numba'-side=100-rank=2-format='coo'-mul]`	N/A	3 ms	N/A
🆕	`test_elemwise[backend='Numba'-side=100-rank=2-format='gcxs'-add]`	N/A	7.9 ms	N/A
🆕	`test_elemwise[backend='Numba'-side=100-rank=2-format='gcxs'-mul]`	N/A	6.8 ms	N/A
🆕	`test_elemwise[backend='Numba'-side=100-rank=3-format='coo'-add]`	N/A	22 ms	N/A
🆕	`test_elemwise[backend='Numba'-side=100-rank=3-format='coo'-mul]`	N/A	9.6 ms	N/A
🆕	`test_elemwise[backend='Numba'-side=100-rank=3-format='gcxs'-add]`	N/A	32.9 ms	N/A
🆕	`test_elemwise[backend='Numba'-side=100-rank=3-format='gcxs'-mul]`	N/A	16.9 ms	N/A
🆕	`test_elemwise[backend='Numba'-side=1000-rank=1-format='coo'-add]`	N/A	3.5 ms	N/A
🆕	`test_elemwise[backend='Numba'-side=1000-rank=1-format='coo'-mul]`	N/A	2.7 ms	N/A
🆕	`test_elemwise[backend='Numba'-side=1000-rank=1-format='gcxs'-add]`	N/A	4.2 ms	N/A
🆕	`test_elemwise[backend='Numba'-side=1000-rank=1-format='gcxs'-mul]`	N/A	3.3 ms	N/A
🆕	`test_elemwise[backend='Numba'-side=1000-rank=2-format='coo'-add]`	N/A	19.9 ms	N/A
🆕	`test_elemwise[backend='Numba'-side=1000-rank=2-format='coo'-mul]`	N/A	8.9 ms	N/A
🆕	`test_elemwise[backend='Numba'-side=1000-rank=2-format='gcxs'-add]`	N/A	28.7 ms	N/A
🆕	`test_elemwise[backend='Numba'-side=1000-rank=2-format='gcxs'-mul]`	N/A	14.1 ms	N/A
...	...	...	...	...

ℹ️ Only the first 20 benchmarks are displayed. Go to the app to view all benchmarks.

hameerabbasi

Great work @DeaMariaLeon! A few suggestions.

benchmarks/conftest.py

benchmarks/test_benchmark_coo.py

hameerabbasi · 2024-09-16T16:13:29Z

It seems the Finch benchmarks don't work: https://github.com/pydata/sparse/actions/runs/10884899040/job/30201213331?pr=772#step:6:142

DeaMariaLeon · 2024-09-17T08:18:18Z

benchmarks/test_benchmark_coo.py

    rng = np.random.default_rng(seed=seed)
    x = sparse.random((m, n), density=DENSITY, format=format, random_state=rng)
    y = sparse.random((n, p), density=DENSITY, format=format, random_state=rng)



All the tests on file test_benchmark_coo.py were meant to fail with Finch.
Finch sparse.random (used to build x & y, line 25 and 26) is different, because it doesn't have the format argument that is used with Numba.

@mtsokol How hard would it be to add this to finch?

You can probably use sparse.asarray(..., format=<format>)

Do we just need a format=nothing default and then a reformatting line inside finch.random?

That'd suffice -- if the format isn't what the format arg demands, reformat.

do we have a way of specifying format currently?

do we have a way of specifying format currently?

asarray supports a format arg: https://github.com/willow-ahrens/finch-tensor/blob/25d5de0c6b0c75120a06c0b1c2ec1568216c71f8/src/finch/tensor.py#L647

hameerabbasi · 2024-09-17T14:05:55Z

benchmarks/test_benchmark_coo.py

+    if hasattr(sparse, "compiled"):
+        operator.matmul = sparse.compiled(operator.matmul)


Suggested change

if hasattr(sparse, "compiled"):

operator.matmul = sparse.compiled(operator.matmul)

f = operator.matmul

if hasattr(sparse, "compiled"):

f = sparse.compiled(f)

Suggested change

if hasattr(sparse, "compiled"):

operator.matmul = sparse.compiled(operator.matmul)

def f(x, y):

return x @ y

if hasattr(sparse, "compiled"):

f = sparse.compiled(f)

hameerabbasi · 2024-09-17T14:08:04Z

benchmarks/test_benchmark_coo.py

+    if hasattr(sparse, "compiled"):
+        operator.matmul = sparse.compiled(operator.matmul)
+
    x @ y  # Numba compilation


Suggested change

x @ y # Numba compilation

f(x, y) # Compilation

hameerabbasi · 2024-09-17T14:13:33Z

benchmarks/test_benchmark_coo.py

    m, n, p = sides

-    if m * n >= max_size or n * p >= max_size:
+    if m * n >= max_size or n * p >= max_size or m * n <= min_size or n * p <= min_size:


Suggested change

if m * n >= max_size or n * p >= max_size or m * n <= min_size or n * p <= min_size:

if m * n >= max_size or n * p >= max_size or m * n * DENSITY <= min_size or n * p * DENSITY <= min_size:

hameerabbasi · 2024-09-17T14:22:05Z

backend should be the first argument in all tests.

wip

2b25d4e

github-actions bot added the internal label Sep 10, 2024

[pre-commit.ci] auto fixes from pre-commit.com hooks

ab21082

for more information, see https://pre-commit.ci

hameerabbasi requested changes Sep 10, 2024

View reviewed changes

benchmarks/conftest.py Outdated Show resolved Hide resolved

benchmarks/test_benchmark_coo.py Outdated Show resolved Hide resolved

benchmarks/test_benchmark_coo.py Show resolved Hide resolved

DeaMariaLeon added 3 commits September 16, 2024 15:22

Correcting conftest and adding Finch compilation

e6eed2e

After pre-commit

f828ba9

Merge remote-tracking branch 'upstream/main' into bench2

f822c24

DeaMariaLeon marked this pull request as ready for review September 16, 2024 13:44

DeaMariaLeon commented Sep 17, 2024

View reviewed changes

hameerabbasi reviewed Sep 17, 2024

View reviewed changes

hameerabbasi mentioned this pull request Sep 30, 2024

ENH: Add counting triangles example #781

Merged

		if hasattr(sparse, "compiled"):
		operator.matmul = sparse.compiled(operator.matmul)

	if m * n >= max_size or n * p >= max_size or m * n <= min_size or n * p <= min_size:
	if m * n >= max_size or n * p >= max_size or m * n * DENSITY <= min_size or n * p * DENSITY <= min_size:

Uh oh!

Conversation

DeaMariaLeon commented Sep 10, 2024

What type of PR is this? (check all applicable)

Related issues

Checklist

Please explain your changes below.

Uh oh!

codspeed-hq bot commented Sep 10, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Merging #772 will not alter performance

Summary

Benchmarks breakdown

Uh oh!

hameerabbasi left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

hameerabbasi commented Sep 16, 2024

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

hameerabbasi commented Sep 17, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

codspeed-hq bot commented Sep 10, 2024 •

edited

Loading