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

Repeat example from the docs is broken #46

Closed
mcabbott opened this issue Aug 21, 2021 · 5 comments · Fixed by #51
Closed

Repeat example from the docs is broken #46

mcabbott opened this issue Aug 21, 2021 · 5 comments · Fixed by #51
Labels
bug Something isn't working

Comments

@mcabbott
Copy link
Owner

julia> M = Array(reshape(1:12, 3,4))
3×4 Matrix{Int64}:
 1  4  7  10
 2  5  8  11
 3  6  9  12

julia> @cast R[r,(n,c)] := M[r,c]^2  (n in 1:3)
ERROR: LoadError: index n appears only on the left

julia> V = [10,20,30];

julia> @cast _[i,j] := V[i]   (j in 1:4)  # simpler version
ERROR: LoadError: index j appears only on the left

julia> @cast _[i,j] := V[i] + 0j   (j in 1:4)  # this works!
3×4 Matrix{Int64}:
 10  10  10  10
 20  20  20  20
 30  30  30  30
@RomeoV
Copy link
Contributor

RomeoV commented Aug 25, 2022

Any progress on this?
I'm running into a similar problem.

x = rand(3, 4)
@test repeat(x, 1, 2) == @cast _[h, (w, 2)] :=  x[h, w]
# or
@test repeat(x, 1, 2) == @cast _[h, (w, j)] :=  x[h, w] (j in 1:2)

(Both fail).
Perhaps the check for "only on the left" should be removed? I think the first version is even nicer though, and is also supported by python's einops library.

@mcabbott
Copy link
Owner Author

Sorry, I haven't yet looked into this. Not so hard, I hope.

@RomeoV
Copy link
Contributor

RomeoV commented Aug 25, 2022

Great. I'm comparing TensorCast.jl to the recent paper published on einops (https://openreview.net/pdf?id=oapKSVM2bcj).
If this is fixed, all examples in Listing 1 can be done with TensorCast.jl and I can provide a pull request with the appropriate tests / examples.

@mcabbott mcabbott added the bug Something isn't working label Sep 3, 2022
@mcabbott
Copy link
Owner Author

mcabbott commented Sep 3, 2022

That sounds great.

One or twice I've tried to borrow documentation from einops... I see there's an ancient notebook currently committed, but not linked from anywhere. Would be nice to do more. And more tests never hurt either.

Some investigation into this repeat problem:

julia> R = randn(3, 3*4);  # write in-place, easier:

julia> @cast R[r,(n,c)] = M[r,c]^2  # (n in 1:3)  # n can be inferred
ERROR: LoadError: index n appears only on the left
Stacktrace:
 [1] checkallseen ...
 [2] _macro(exone::Expr, extwo::Nothing, exthree::Nothing; call::TensorCast.CallInfo, dict::Dict{Any, Any})
   @ TensorCast ~/.julia/dev/TensorCast/src/macro.jl:199

# comment out check on line 199 and it works:

julia> @cast R[r,(n,c)] = M[r,c]^2  # (n in 1:3)
3×3×4 Array{Float64, 3}:
[:, :, 1] =
 1.0  1.0  1.0
 4.0  4.0  4.0
...

julia> R  # line above returns the wrong thing, reshaped rather than R itself:
3×12 Matrix{Float64}:
 1.0  1.0  1.0  16.0  16.0  16.0  49.0  49.0  49.0  100.0  100.0  100.0
 4.0  4.0  4.0  25.0  25.0  25.0  64.0  64.0  64.0  121.0  121.0  121.0
 9.0  9.0  9.0  36.0  36.0  36.0  81.0  81.0  81.0  144.0  144.0  144.0

# out-of-place again:

julia> @cast R[r,(n,c)] := M[r,c]^2  (n in 1:3)
ERROR: DimensionMismatch: new dimensions (3, 12) must be consistent with array size 12

julia> @pretty @cast R[r,(n,c)] := M[r,c]^2  (n in 1:3)
begin
    @boundscheck ndims(M) == 2 || throw(ArgumentError("expected a 2-tensor M[r, c]"))
    local (ax_c, ax_n, ax_r) = (axes(M, 2), OneTo(3), axes(M, 1))
    local spider = transmute(M, Val((1, nothing, 2)))
    R = reshape(@__dot__(spider ^ 2), (ax_r, star(ax_n, ax_c)))
end

julia> @cast R[r,(n,c)] := M[r,c]^2 + 0n  (n in 1:3)
3×12 Matrix{Int64}:
 1  1  1  16  16  16  49  49  49  100  100  100
 4  4  4  25  25  25  64  64  64  121  121  121
 9  9  9  36  36  36  81  81  81  144  144  144

To make the new dimension, it needs to do something like .+ 0 .* transpose(ax_n) in the broadcast (as in the last expression). There must be, or have been, logic for this, somewhere...

According to git blame I added this to docs here between 0.4.0 and 0.4.1, but it doesn't run on either of those versions.

Ah now I found a branch: master...repeat

@mcabbott
Copy link
Owner Author

mcabbott commented Sep 3, 2022

Note BTW that @cast _[h, (w, 2)] := x[h, w] will probably never work, but would ideally have a better error.

This is because @cast _[h] := x[h, 1] is already the first column, a constant index. Plausibly @cast _[h, j] := x[h, (j, 1)] (j in 1:2) should then extract some subset of the columns.

The way to specify sizes used to be something like @cast _[h, (w, j:2)] := x[h, w], or else j:2 after the expression, notation chosen to be compact. But this was removed when everything was upgraded to allow offsets everywhere.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants