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

Calling ncon with a view throws MethodError #119

Open
votroto opened this issue Jul 2, 2022 · 3 comments
Open

Calling ncon with a view throws MethodError #119

votroto opened this issue Jul 2, 2022 · 3 comments

Comments

@votroto
Copy link

votroto commented Jul 2, 2022

Is it possible to support views into arrays? It seems the api might be too specific.

e.g. a Matrix works as expected:

A = [1 2 3; 4 5 6]
B = [0, 1, 0]
C = [1, 1]

@show ncon([A, B, C], [[2,1], [1], [2]])

An equivalent view into a matrix throws a MethodError: no method matching sview error

A = view([1 2 3 1; 4 5 6 1; 7 8 9 1], [1,2], [1,2,3])
B = [0, 1, 0]
C = [1, 1]

@show ncon([A, B, C], [[2,1], [1], [2]])

Simply doing collect works as expected.

A = collect(view([1 2 3 1; 4 5 6 1; 7 8 9 1], [1,2], [1,2,3]))
B = [0, 1, 0]
C = [1, 1]

@show ncon([A, B, C], [[2,1], [1], [2]])

Thank You

@Jutho
Copy link
Owner

Jutho commented Jul 3, 2022

If you were to replace A with
A = view([1 2 3 1; 4 5 6 1; 7 8 9 1], 1:2, 1:3)
you would find that it works. The general rule is that views are supported, if they are sliced with ranges. If you slice with arbitrary vectors, you could do things like
view(some_array, [1,3,4,13,14,6], ...)
with completely unpredictable memory layout for the elements of the resulting view. Technically, such a view would not have a strided memory layout. Only views with a strided memory layout are supported, which are those that you can make by only using ranges (and single integers, and colons) for indexing/slicing.

@votroto
Copy link
Author

votroto commented Jul 3, 2022

Rigth, then I guess what I'm asking for is a fallback implementation for objects without a strided memory layout. Tensor operations are still well defined even on general views, right?

Forgetting the performance implications for a moment, would that be possible?

@Jutho
Copy link
Owner

Jutho commented Jul 3, 2022

That would indeed be possible, a simple implementation based on loops could indeed be provided. The need for this was never very high. There are also some tricky (but not insurmountable) issues with dispatch.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants