-
Notifications
You must be signed in to change notification settings - Fork 15
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
Is eye() specific to row major layouts? #90
Comments
that's definitely a bug it should behave the same way for row- vs col-major (your expectation seems correct) |
looking into this right now, so far i couldn't reproduce with "pure" xtensor. checking xtensor-r now! |
Ok, I looked into this and I think this is related to R's way of printing. E.g. check out:
and this is true and works. However, the way R prints multidimensional arrays is very awkward, and leads to this confusion. What do you think @DavisVaughan ? Sorry, again, that it took so long to get back to this. |
Ok, just thought about it for two more seconds, and i guess what you want is to have an
Which I guess makes sense if one would do column major style broadcasting for things like matrix product etc, or anything else. Now this is again leading to more fundamental questions ... wether we could / should support col-major "broadcasting". I fear right now we don't have the manpower for that and it's also not clear that it would bring a lot of advantages. On the other hand, given all the code we already have for row-major broadcasting, it should not be so far out to do it, but it adds some complexity to the overall code base... |
Ah this looks heavily related to the prepend / append of new dimension conversation in #57. Where you have To me, both mean "second element of first dimension, second element of second dimension, first element of third dimension" I don't think it has to do with the way R prints. To be fair, because of these prepend/append of dimension differences (which bleeds into how things print), I think numpy array printing is highly confusing, with R printing being more intuitive. About to get on a plane! Can chat more later today |
The new equality you show above looks correct to me! |
I think |
I can't tell if this is expected behavior of
eye()
or not. Is there a column major option for it?I dont really see anything here about row/column major, so I honestly have no idea
https://github.com/QuantStack/xtensor/blob/master/include/xtensor/xbuilder.hpp#L305
Created on 2019-01-09 by the reprex package (v0.2.1.9000)
The text was updated successfully, but these errors were encountered: