-
Notifications
You must be signed in to change notification settings - Fork 152
Views of SArray may use getindex to avoid the SubArray wrapper #908
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I generally prefer not giving special treatment to SArray
- if there were a generic way of doing this (like a trait for immutable arrays) that would be preferable.
On the other hand this is quite practical and seems harmless to merge.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks great now, could you just bump version to 1.2.0? I'll merge and tag a new release.
Rebased on master and bumped version to 1.2.0 |
This PR caused a difference between using a using ForwardDiff, LinearAlgebra, StaticArrays
foo2(A) = SVector{4}(norm(r, 2) for r = eachrow(A))
function foo1!(out, x; μ = 0.8 / √2)
λ = SVector{3}(@view x[1:3])
K = SMatrix{3,3}(@view x[4:12])
A_λ = [ 1 0 -1 0 ;
0 1 0 -1 ;
μ μ μ μ ]
out[1:4] = A_λ' * λ + foo2(A_λ' * K)
out
end
function foo1_s!(out, x; μ = 0.8 / √2)
λ = SVector{3}(@view x[1:3])
K = SMatrix{3,3}(@view x[4:12])
A_λ = @SMatrix [ 1 0 -1 0 ;
0 1 0 -1 ;
μ μ μ μ ]
out[1:4] = A_λ' * λ + foo2(A_λ' * K)
out
end
x = zeros(12);
out = zeros(4);
display(ForwardDiff.jacobian(foo1!, out, x))
display(ForwardDiff.jacobian(foo1_s!, out, x))
nothing and before this PR they returned the same value, after it, the one with |
The difference is in one check that LinearAlgebra's Probably StaticArrays.jl needs to have this check too? |
Thank you for finding this PR as the breaking changes, @KristofferC. And thank you @mateuszbaran for your reply. Should I create a new issue to track this? |
Sounds like a good idea. |
Since
SArrays
are immutable and support allocation-less vector indexing for certain index types, views ofSArray
s constructed with such indices do not need to retain theSubArray
wrapper and may just return anotherSArray
obtained through indexing. Fixes #892.After this PR: