search

willow-ahrens / finch-tensor

Sparse Tensors in Python and more! Datastructure-Driven Array Programming Language
MIT License
8 stars 3 forks source link

Make `nonzero` return Tensors #55

Closed mtsokol closed 4 months ago

mtsokol commented 4 months ago

Hi @hameerabbasi,

This is a small tweak to make sure nonzero returns Tensor's instead of NumPy arrays.

willow-ahrens commented 4 months ago

Is this the desired approach? I'm fine with this but I wonder if users would want a wrapped array when they call these destructuring operators. Usually they call this when the arrays are moving to a different library or framework.

willow-ahrens commented 4 months ago

Hameer, you've been right before when it comes to conventions like this, so I'll defer to you here. This struck me as strange though since most of the time people call this when they "just want the data"

hameerabbasi commented 4 months ago

Hameer, you've been right before when it comes to conventions like this, so I'll defer to you here. This struck me as strange though since most of the time people call this when they "just want the data"

Well, what I'd do is compute arr == zero_scalar as a Tensor with a dtype of bool, materialize it as a canonicalized COO, and pull out the coords.

mtsokol commented 4 months ago

I wrapped it because array-api-tests expects results of functions of the same type as inputs. So, Tensor in, Tensor out. When Tensor is an input and NumPy is an output it flags it as an error.

hameerabbasi commented 4 months ago

I wrapped it because array-api-tests expects results of functions of the same type as inputs. So, Tensor in, Tensor out. When Tensor is an input and NumPy is an output it flags it as an error.

Yes, we should definitely have "this library type" in/out.

mtsokol commented 4 months ago

Hameer, you've been right before when it comes to conventions like this, so I'll defer to you here. This struck me as strange though since most of the time people call this when they "just want the data"

Well, what I'd do is compute arr == zero_scalar as a Tensor with a dtype of bool, materialize it as a canonicalized COO, and pull out the coords.

We already have nonzero implementation on the Finch side, which we're calling here.

willow-ahrens commented 4 months ago

Ah, I see. No worries, we can leave as is if the Array API requires it. Just thought I'd register that this is usually not what the user would expect from a function like this. A related function is "find". In Scipy, "find" does not return a tuple of scipy.sparse arrays, it just returns ndarray. https://docs.scipy.org/doc/scipy/reference/generated/scipy.sparse.find.html#scipy.sparse.find

willow-ahrens commented 4 months ago

I think most people will find themselves unwrapping the results of this function

willow-ahrens commented 4 months ago

Maybe if find is not in the API standard, we could introduce a function called find that just returns ndarrays.