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

norm of StaticArray inaccurate #913

Closed
baggepinnen opened this issue May 19, 2021 · 6 comments · Fixed by #975
Closed

norm of StaticArray inaccurate #913

baggepinnen opened this issue May 19, 2021 · 6 comments · Fixed by #975
Labels
numerical-robustness numerical robustness and accuracy

Comments

@baggepinnen
Copy link

baggepinnen commented May 19, 2021

I have traced a bug in my code to the fact that norm(::StaticArray) is considerably less accurate than norm(::Array). In code working with rotations, the norm of a small vector, length 2-4, is often used required to very high precision.

To verify that the problem is the naive implementation of norm for static arrays, I modified the norm function like so

@generated function _norm(::Size{S}, a::StaticArray) where {S}
    if prod(S) == 0
        return :(zero(real(eltype(a))))
    end
    
    expr = :(abs2(a[1]))
    for j = 2:prod(S)
        expr = :($expr + abs2(a[$j]))
    end
    
    return quote
        return norm(Array(a))    # <------------- convert to standard array and return norm of this instead 
        $(Expr(:meta, :inline))
        @inbounds return sqrt($expr)
    end
end

after which the problem goes away.

Would it be feasible to have an implementation of norm that is more accurate, at least for small vectors?

MWE:

julia> using StaticArrays, LinearAlgebra

julia> a = SA[0, 1e-180]
2-element SVector{2, Float64} with indices SOneTo(2):
 0.0
 1.0e-180

julia> norm(a)
0.0

julia> norm(Array(a))
1.0e-180
@andyferris
Copy link
Member

OK, so looking at LinearAlgebra, our approach should be the same as LinearAlgebra.generic_norm2, at least for the happy path.

For smaller Vector{Float64} it calls out to BLAS. I wonder what approach it uses?

Also it would be helpful if you posted an example vector with the two results in the OP.

@baggepinnen
Copy link
Author

I added a simple example to the OP where the static version fails

@andyferris
Copy link
Member

Oh that one.

We could just port the “unhappy path” from LinearAlgebra.generic_norm2, right?

What do other users think? This will introduce a calculation of the maximum value, a branch, divisions, etc. It might be a significant slowdown and we’ve previously made different choices of speed vs accuracy to Base. I’m not sure this issue is so relevant for computer graphics or geography/geodesy or robotics or other fields I’m aware of this package being quite useful.

If someone wants to run some benchmarks (I’d recommend finding the norms of a Vector of SVectors) that would inform the discussion.

@baggepinnen
Copy link
Author

baggepinnen commented May 20, 2021

The issue occured in a robotics context, where the vector was used to represent rotations. Using differentiating a function with this calculation with ForwardDiff produced NaNs due to norm(v::SVector), so I would say it's relevant. This is the exact application of norm that caused the failure
https://github.com/baggepinnen/Robotlib.jl/blob/master/src/utils.jl#L94
calculating the exponential of a skew-symmetric matrix exp(w) to obtain a rotation matrix.

RigidBodyDynamics.jl have a very similar implementation of exp which is probably susceptible to the same problem.
https://github.com/JuliaRobotics/RigidBodyDynamics.jl/blob/0f42d2900174adc3c03911d217fd48a598c69bb8/src/spatial/spatialmotion.jl#L316

baggepinnen added a commit to baggepinnen/RigidBodyDynamics.jl that referenced this issue May 20, 2021
This PR is mostly a FYI regarding JuliaArrays/StaticArrays.jl#913
Depending on whether or not that issue is closed, you may want to switch to explicitly calling `generic_norm2` on static vectors to circumvent the accuracy issue. The issue arises when combining static arrays with ForwardDiff, in our case it occured in `exp(::SkewSymmetric)`. 

I did some benchmarking, and `generic_norm2` is faster than `norm` for all standard arrays up to at least length 9. For static arrays, it appears to do okay as well, while avoiding the accuracy issue.
```julia
julia> a
3-element SVector{3, Int64} with indices SOneTo(3):
 1
 2
 3

julia> @Btime norm($(Ref(a))[]) # standard norm of static array
  5.135 ns (0 allocations: 0 bytes)
3.74166

julia> @Btime norm($(Ref(Vector(a)))[]) # standard norm of  array
  8.545 ns (0 allocations: 0 bytes)
3.74166

julia> @Btime LinearAlgebra.generic_norm2($(Ref((a)))[]) # generic_norm2 of static array
  4.490 ns (0 allocations: 0 bytes)
3.74166

julia> @Btime LinearAlgebra.generic_norm2($(Ref(Vector(a)))[]) # generic_norm2 of array
  7.671 ns (0 allocations: 0 bytes)
3.74166

```
@andyferris
Copy link
Member

I see, you represent the rotation in the Lie algebra (not the group a la Rotations.jl) and thus it can be small.

(Also worth noting that for this test the 1-norm or infinite-norm would also suffice).

@mateuszbaran mateuszbaran added the numerical-robustness numerical robustness and accuracy label Jun 17, 2021
@ferrolho
Copy link

ferrolho commented Oct 14, 2021

I recently opened two issues, JuliaDiff/ForwardDiff.jl#547 and #963, which I think are related to this.

ferrolho pushed a commit to ferrolho/RigidBodyDynamics.jl that referenced this issue Nov 5, 2024
This PR is mostly a FYI regarding JuliaArrays/StaticArrays.jl#913
Depending on whether or not that issue is closed, you may want to switch to explicitly calling `generic_norm2` on static vectors to circumvent the accuracy issue. The issue arises when combining static arrays with ForwardDiff, in our case it occured in `exp(::SkewSymmetric)`.

I did some benchmarking, and `generic_norm2` is faster than `norm` for all standard arrays up to at least length 9. For static arrays, it appears to do okay as well, while avoiding the accuracy issue.
```julia
julia> a
3-element SVector{3, Int64} with indices SOneTo(3):
 1
 2
 3

julia> @Btime norm($(Ref(a))[]) # standard norm of static array
  5.135 ns (0 allocations: 0 bytes)
3.74166

julia> @Btime norm($(Ref(Vector(a)))[]) # standard norm of  array
  8.545 ns (0 allocations: 0 bytes)
3.74166

julia> @Btime LinearAlgebra.generic_norm2($(Ref((a)))[]) # generic_norm2 of static array
  4.490 ns (0 allocations: 0 bytes)
3.74166

julia> @Btime LinearAlgebra.generic_norm2($(Ref(Vector(a)))[]) # generic_norm2 of array
  7.671 ns (0 allocations: 0 bytes)
3.74166

```
ferrolho pushed a commit to ferrolho/RigidBodyDynamics.jl that referenced this issue Nov 6, 2024
This PR is mostly a FYI regarding JuliaArrays/StaticArrays.jl#913
Depending on whether or not that issue is closed, you may want to switch to explicitly calling `generic_norm2` on static vectors to circumvent the accuracy issue. The issue arises when combining static arrays with ForwardDiff, in our case it occured in `exp(::SkewSymmetric)`.

I did some benchmarking, and `generic_norm2` is faster than `norm` for all standard arrays up to at least length 9. For static arrays, it appears to do okay as well, while avoiding the accuracy issue.
```julia
julia> a
3-element SVector{3, Int64} with indices SOneTo(3):
 1
 2
 3

julia> @Btime norm($(Ref(a))[]) # standard norm of static array
  5.135 ns (0 allocations: 0 bytes)
3.74166

julia> @Btime norm($(Ref(Vector(a)))[]) # standard norm of  array
  8.545 ns (0 allocations: 0 bytes)
3.74166

julia> @Btime LinearAlgebra.generic_norm2($(Ref((a)))[]) # generic_norm2 of static array
  4.490 ns (0 allocations: 0 bytes)
3.74166

julia> @Btime LinearAlgebra.generic_norm2($(Ref(Vector(a)))[]) # generic_norm2 of array
  7.671 ns (0 allocations: 0 bytes)
3.74166

```
ferrolho pushed a commit to ferrolho/RigidBodyDynamics.jl that referenced this issue Nov 6, 2024
This PR is mostly a FYI regarding JuliaArrays/StaticArrays.jl#913
Depending on whether or not that issue is closed, you may want to switch to explicitly calling `generic_norm2` on static vectors to circumvent the accuracy issue. The issue arises when combining static arrays with ForwardDiff, in our case it occured in `exp(::SkewSymmetric)`.

I did some benchmarking, and `generic_norm2` is faster than `norm` for all standard arrays up to at least length 9. For static arrays, it appears to do okay as well, while avoiding the accuracy issue.
```julia
julia> a
3-element SVector{3, Int64} with indices SOneTo(3):
 1
 2
 3

julia> @Btime norm($(Ref(a))[]) # standard norm of static array
  5.135 ns (0 allocations: 0 bytes)
3.74166

julia> @Btime norm($(Ref(Vector(a)))[]) # standard norm of  array
  8.545 ns (0 allocations: 0 bytes)
3.74166

julia> @Btime LinearAlgebra.generic_norm2($(Ref((a)))[]) # generic_norm2 of static array
  4.490 ns (0 allocations: 0 bytes)
3.74166

julia> @Btime LinearAlgebra.generic_norm2($(Ref(Vector(a)))[]) # generic_norm2 of array
  7.671 ns (0 allocations: 0 bytes)
3.74166

```
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
numerical-robustness numerical robustness and accuracy
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants