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

MAINT: Re-enable COO tests after fix #814

Merged
merged 1 commit into from
Nov 27, 2024
Merged

MAINT: Re-enable COO tests after fix #814

merged 1 commit into from
Nov 27, 2024

Conversation

mtsokol
Copy link
Collaborator

@mtsokol mtsokol commented Nov 25, 2024

Hi @hameerabbasi,

As described in llvm/llvm-project#116012 (comment) this PR re-enables COO tests and makes (temporarily) COO instantiation copy-full.

Addition ops with COO still fail though.

@mtsokol mtsokol self-assigned this Nov 25, 2024
Copy link

codspeed-hq bot commented Nov 25, 2024

CodSpeed Performance Report

Merging #814 will degrade performances by 32.16%

Comparing enable-coo-tests (e4e4687) with main (c0b2b45)

Summary

⚡ 2 improvements
❌ 1 regressions
✅ 337 untouched benchmarks

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

Benchmarks breakdown

Benchmark main enable-coo-tests Change
test_index_fancy[side=100-rank=1-format='coo'] 1.5 ms 1.1 ms +27.96%
test_index_slice[side=100-rank=2-format='gcxs'] 2.4 ms 3.5 ms -32.16%
test_elemwise[f=<built-in function gt>-backend='Finch'-side=1000] 62 ms 1.5 ms ×42

Copy link
Collaborator

@hameerabbasi hameerabbasi left a comment

Choose a reason for hiding this comment

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

Thanks for the deep dive here, @mtsokol

@hameerabbasi
Copy link
Collaborator

Thanks for the deep dive here, @mtsokol!

@mtsokol mtsokol merged commit 85e6e69 into main Nov 27, 2024
18 of 19 checks passed
@mtsokol mtsokol deleted the enable-coo-tests branch November 27, 2024 10:40
@hameerabbasi
Copy link
Collaborator

Seems like this doesn't fix test_add and test_add_dense_sparse, I just tried removing the xfails and it crapped out.

@mtsokol
Copy link
Collaborator Author

mtsokol commented Nov 28, 2024

Right, the PR's title is imprecise. It enabled test_coo_3d_format and test_asformat only. test_add is still broken.

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

Successfully merging this pull request may close these issues.

2 participants