finch-tensor / finch-tensor-python

Sparse and Structured Tensor Programming in Python
MIT License
8 stars 3 forks source link

Slicing and indexing for `Tensor` #12

Closed mtsokol closed 7 months ago

mtsokol commented 8 months ago

Hi @willow-ahrens,

This is a tracking issue for implementing slicing and indexing functionality for Tensor class.

Right now I'm mapping objects that can be used for indexing from Python to Julia:

JuliaError: ArgumentError: invalid index: <py slice(None, None, None)> of type Py Stacktrace: [1] to_index(i::Py) @ Base ./indices.jl:300 [2] to_index(A::Tensor{DenseLevel{Int64, DenseLevel{Int64, DenseLevel{Int64, ElementLevel{0, Int64, Int64, Vector{Int64}}}}}}, i::Py) @ Base ./indices.jl:277 [3] _to_indices1(A::Tensor{DenseLevel{Int64, DenseLevel{Int64, DenseLevel{Int64, ElementLevel{0, Int64, Int64, Vector{Int64}}}}}}, inds::Tuple{Base.OneTo{Int64}, Base.OneTo{Int64}, Base.OneTo{Int64}}, I1::Py) @ Base ./indices.jl:359 [4] to_indices @ ./indices.jl:354 [inlined] [5] to_indices @ ./indices.jl:344 [inlined] [6] getindex(::Tensor{DenseLevel{Int64, DenseLevel{Int64, DenseLevel{Int64, ElementLevel{0, Int64, Int64, Vector{Int64}}}}}}, ::Py, ::Py, ::Py) @ Finch ~/.julia/packages/Finch/2wGtC/src/interface/index.jl:28 [7] pyjlanygetitem(self::Tensor{DenseLevel{Int64, DenseLevel{Int64, DenseLevel{Int64, ElementLevel{0, Int64, Int64, Vector{Int64}}}}}}, k::Py)

where Python slice is accepted by Julia arrays:
```julia
r = jl.rand(2,3,4)
r[:,:,:]
willow-ahrens commented 8 months ago

If the slice does not have a step of 1 I think we should try to treat it in Finch as if it's just some array. If the slice has a step of 1 we do have a special case for that, but I think you would need to convert to the julia range type. Could you convert the python slices to julia ranges?

willow-ahrens commented 8 months ago

I would indeed construct elipsis manually

mtsokol commented 8 months ago

Could you convert the python slices to julia ranges?

@willow-ahrens Oh, it looks that Julia's ranges can fully replace Python slices, thanks!:

In [110]: jl.size(arr_finch._obj.body[jl.range(start=1, step=2, stop=4),jl.range(length=2),jl.range(start=2, stop=3)])
Out[110]: (2, 2, 2)
willow-ahrens commented 8 months ago

I wonder why it wasn't captured in the automatic conversions listed here: https://juliapy.github.io/PythonCall.jl/stable/conversion-to-julia/

mtsokol commented 8 months ago

I think there's a conversion of Python's range to Julia's range, where both are iterable. I think that Python's slice object isn't converted.

willow-ahrens commented 8 months ago

Ah I see. There's a way to add conversions, though I'm not sure whether you think we should do that ourselves or through JuliaCall