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 Array.asformat method and add back reshape function. #800

Merged
merged 14 commits into from
Nov 13, 2024

Conversation

hameerabbasi
Copy link
Collaborator

  • Fix signed/unsigned integer naming mix-up.
  • Add mechanism for detecting output format.

@hameerabbasi hameerabbasi added enhancement Indicates new feature requests maintenance labels Oct 24, 2024
Copy link

codspeed-hq bot commented Oct 24, 2024

CodSpeed Performance Report

Merging #800 will not alter performance

Comparing hameerabbasi:add-ops (e561587) with main (e111324)

Summary

✅ 340 untouched benchmarks

@hameerabbasi hameerabbasi changed the title Add back removed ops Add back reshape Nov 4, 2024
@hameerabbasi hameerabbasi marked this pull request as ready for review November 4, 2024 09:51
@hameerabbasi hameerabbasi enabled auto-merge (squash) November 4, 2024 09:51
Copy link
Collaborator

@mtsokol mtsokol left a comment

Choose a reason for hiding this comment

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

As #799 is now merged let's rebase it and also include COO in the determine_format function. I think that it could be COO+COO=COO and CSR+CSR=CSR etc.

sparse/mlir_backend/_common.py Show resolved Hide resolved
sparse/mlir_backend/_ops.py Outdated Show resolved Hide resolved
sparse/mlir_backend/_ops.py Outdated Show resolved Hide resolved
sparse/mlir_backend/levels.py Show resolved Hide resolved
sparse/mlir_backend/levels.py Show resolved Hide resolved
@hameerabbasi
Copy link
Collaborator Author

hameerabbasi commented Nov 11, 2024

As #799 is now merged let's rebase it and also include COO in the determine_format function. I think that it could be COO+COO=COO and CSR+CSR=CSR etc.

Right now COO+COO=DCSR (as both levels are sparse for COO, and I'd like to avoid special cases as much as possible). However; I added an asformat method to convert between formats if we need to.

Copy link
Collaborator

@mtsokol mtsokol left a comment

Choose a reason for hiding this comment

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

The progress looks good! One more round of comments.

pixi.toml Show resolved Hide resolved
sparse/mlir_backend/_ops.py Outdated Show resolved Hide resolved
sparse/mlir_backend/_ops.py Outdated Show resolved Hide resolved
sparse/mlir_backend/tests/test_simple.py Outdated Show resolved Hide resolved
sparse/mlir_backend/tests/test_simple.py Outdated Show resolved Hide resolved
sparse/mlir_backend/_ops.py Show resolved Hide resolved
sparse/mlir_backend/levels.py Outdated Show resolved Hide resolved
Copy link
Collaborator

@mtsokol mtsokol left a comment

Choose a reason for hiding this comment

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

I think it's the last round of review - I think test_reshape should be refined to cover most of reasonable scenarios.

And let's push another commit instead of force push.

sparse/mlir_backend/tests/test_simple.py Outdated Show resolved Hide resolved
sparse/mlir_backend/tests/test_simple.py Outdated Show resolved Hide resolved
sparse/mlir_backend/tests/test_simple.py Show resolved Hide resolved
@hameerabbasi hameerabbasi changed the title Add back reshape Add Array.asformat method and dd back reshape function. Nov 13, 2024
@hameerabbasi hameerabbasi changed the title Add Array.asformat method and dd back reshape function. Add Array.asformat method and add back reshape function. Nov 13, 2024
Copy link
Collaborator

@mtsokol mtsokol left a comment

Choose a reason for hiding this comment

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

LGTM!

@hameerabbasi hameerabbasi merged commit 4b491b5 into pydata:main Nov 13, 2024
20 of 22 checks passed
@hameerabbasi hameerabbasi deleted the add-ops branch November 13, 2024 10:39
@hameerabbasi
Copy link
Collaborator Author

Thanks for the thorough review, @mtsokol!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Indicates new feature requests maintenance
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants