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

Use vector of indexes #49

Closed
jariji opened this issue Mar 2, 2022 · 3 comments
Closed

Use vector of indexes #49

jariji opened this issue Mar 2, 2022 · 3 comments
Labels
wontfix This will not be worked on

Comments

@jariji
Copy link

jariji commented Mar 2, 2022

I wanted to reduce into a 2-vector by using the indexes stored in ixs. Currently it seems to ignore ixs. What do you think about this idea?

using TensorCast
vs = [[10,20,30,40,50,60] .+ rand(6) for _ in 1:10]
ixs = [1,2,1,2,1,2]
@reduce v[ixs[j]] := mean(i) vs[i][j]
@mcabbott
Copy link
Owner

mcabbott commented Mar 2, 2022

This should probably be an error. Right now it does not notice the indexing on the left, and treats it like v[j]:

julia> v  @reduce v2[j] := mean(i) vs[i][j]
true

julia> @pretty @reduce v[ixs[j]] := mean(i) vs[i][j]
begin
    @boundscheck vs isa Tuple || (ndims(vs) == 1 || throw(ArgumentError("expected a vector or tuple vs[i]")))
    local dragonfly = transmute(stack(vs), Val((2, 1)))
    v = dropdims(mean(dragonfly, dims = 1), dims = 1)
end

The package used to support things like @cast _[i] := first(vs)[ixs[i]], indexing on the right. But this was complicated & fragile, and I removed it at some point, in favour of using Tullio for such things.

Indexing on the left is messier, as it's not entirely clear whether you want points with the same ixs[j] to overwrite or accumulate? Tullio's current behaviour is to overwrite, but that depends on what order it iterates in. I think it should probably be made an error, like mcabbott/Tullio.jl#136 :

julia> @tullio v[ixs[j]] := vs[i][j] / 10
2-element Vector{Float64}:
 50.51324364045704
 60.28909109465455

@jariji
Copy link
Author

jariji commented Mar 3, 2022

Indexing on the left is messier, as it's not entirely clear whether you want points with the same ixs[j] to overwrite or accumulate

If I understand correctly, overwriting is a special case of aggregation using last, so maybe there is a way for the user to specify what they want.

@mcabbott
Copy link
Owner

mcabbott commented Sep 3, 2022

I agree this could be given meanings. However I'm hoping not to add too many more features to this package! For now at least it shouldn't silently do the wrong thing.

@mcabbott mcabbott added the wontfix This will not be worked on label Sep 3, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
wontfix This will not be worked on
Projects
None yet
Development

No branches or pull requests

2 participants