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

Fix CSC/CSR .mT fill-value result #849

Merged
merged 2 commits into from
Mar 13, 2025
Merged

Fix CSC/CSR .mT fill-value result #849

merged 2 commits into from
Mar 13, 2025

Conversation

mtsokol
Copy link
Collaborator

@mtsokol mtsokol commented Mar 13, 2025

Hi @hameerabbasi,

It looks like transposing CSC/CSR drops fill-value. Here's a fix.

@mtsokol mtsokol requested a review from hameerabbasi March 13, 2025 11:02
@mtsokol mtsokol self-assigned this Mar 13, 2025
hameerabbasi
hameerabbasi previously approved these changes Mar 13, 2025
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, @mtsokol!

@hameerabbasi hameerabbasi enabled auto-merge (squash) March 13, 2025 11:11
@hameerabbasi
Copy link
Collaborator

Ouch -- looks like the array API tests fail for Finch. Hmm; let's put that into a separate PR to be merged quickly instead of bundling it with #848?

@hameerabbasi
Copy link
Collaborator

Looks like it's just one test -- that can be bundled and we can merge-commit this PR instead of squash.

@hameerabbasi hameerabbasi disabled auto-merge March 13, 2025 11:15
@hameerabbasi hameerabbasi enabled auto-merge March 13, 2025 11:15
@mtsokol
Copy link
Collaborator Author

mtsokol commented Mar 13, 2025

Looks like it's just one test -- that can be bundled and we can merge-commit this PR instead of squash.

Done!

@hameerabbasi hameerabbasi merged commit 851e82c into main Mar 13, 2025
17 of 18 checks passed
@hameerabbasi hameerabbasi deleted the ms/csr-csc-mT-fix branch March 13, 2025 11:47
Copy link

codspeed-hq bot commented Mar 13, 2025

CodSpeed Performance Report

Merging #849 will degrade performances by 44.29%

Comparing ms/csr-csc-mT-fix (769a5cd) with main (60c552f)

Summary

❌ 2 regressions
✅ 338 untouched benchmarks

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

Benchmarks breakdown

Benchmark BASE HEAD Change
test_index_fancy[side=100-rank=1-format='coo'] 757.1 µs 1,359 µs -44.29%
test_index_slice[side=100-rank=2-format='gcxs'] 2 ms 2.8 ms -27.28%

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.

2 participants