pydata / sparse

Sparse multi-dimensional arrays for the PyData ecosystem
https://sparse.pydata.org
BSD 3-Clause "New" or "Revised" License
604 stars 127 forks source link

MNT: Move constructors to a direct style #773

Closed mtsokol closed 2 months ago

mtsokol commented 2 months ago

Hi @hameerabbasi,

This PR moves MLIR constructors to the direct style (with: sparse-assembler{direct-out=true}). I think it's more concise with this approach - _constructors.py LOC reduced by 50%, to 250.

Now all Tensors created from NumPy arrays or SciPy sparse arrays are owned and freed by the python side.

The ones that are results of MLIR ops (like add right now) are allocated by MLIR and owns_memory in Tensor class is set to False.

I haven't figured out yet how to correctly free MLIR allocated memrefs, (tried with libc.free) as I think there's still a memory leak after sparse.add(tensor_a, tensor_b).

codspeed-hq[bot] commented 2 months ago

CodSpeed Performance Report

Merging #773 will degrade performances by 51.96%

Comparing direct-style (ef3e62e) with main (47b246b)

Summary

โŒ 2 (๐Ÿ‘ 2) regressions โœ… 338 untouched benchmarks

Benchmarks breakdown

Benchmark main direct-style Change
๐Ÿ‘ test_index_fancy[side=100-rank=1-format='coo'] 680 ยตs 1,415.4 ยตs -51.96%
๐Ÿ‘ test_index_slice[side=100-rank=2-format='gcxs'] 2.2 ms 2.5 ms -12.61%
mtsokol commented 2 months ago

@hameerabbasi I applied all your review comments.