JuliaAstro / AstroLib.jl

Bundle of small astronomical and astrophysical routines.
http://juliaastro.github.io/AstroLib.jl/stable/
Other
79 stars 23 forks source link

[WIP] Use static arrays #46

Closed giordano closed 7 years ago

giordano commented 7 years ago

Fix #45.

This is marked as work-in-progress because I'm not sure I made all possible changes. Feel free to report any missing array to change.

codecov-io commented 7 years ago

Codecov Report

Merging #46 into master will increase coverage by 0.06%. The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #46      +/-   ##
==========================================
+ Coverage   99.81%   99.87%   +0.06%     
==========================================
  Files          76       76              
  Lines        1581     1554      -27     
==========================================
- Hits         1578     1552      -26     
+ Misses          3        2       -1
Impacted Files Coverage Δ
src/common.jl 100% <ø> (ø) :arrow_up:
src/gal_uvw.jl 100% <ø> (ø) :arrow_up:
src/precess_cd.jl 100% <100%> (ø) :arrow_up:
src/helio.jl 100% <100%> (ø) :arrow_up:
src/moonpos.jl 100% <100%> (ø) :arrow_up:
src/nutate.jl 100% <100%> (ø) :arrow_up:
src/premat.jl 100% <100%> (ø) :arrow_up:
src/mag2geo.jl 100% <100%> (ø) :arrow_up:
src/baryvel.jl 100% <100%> (ø) :arrow_up:
src/sixty.jl 100% <100%> (ø) :arrow_up:
... and 22 more

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update c8358fc...170d785. Read the comment docs.

TestSubjector commented 7 years ago

Perhaps the following changes?

In bprecess.jl, change line 42 to

s = Vector{T}(s1)

Additionally, since premat() returns an array; either convert it to static or convert the variable that receives the return value of premat (in precess.jl and baryvel.jl) to static?

Small question :- Does StaticArrays give a significant boost to the performance of the code?

giordano commented 7 years ago

Thanks for looking into it!

In bprecess.jl, change line 42 to

s = Vector{T}(s1)

s will be changed, doing what you suggest would change s1 as well, this is what I wanted to avoid with the copy. See this example:

julia> s = randn(3)
3-element Array{Float64,1}:
  0.593006
 -1.79782 
  0.592398

julia> s1 = Vector{Float64}(s)
3-element Array{Float64,1}:
  0.593006
 -1.79782 
  0.592398

julia> s1[1] = -5
-5

julia> s
3-element Array{Float64,1}:
 -5.0     
 -1.79782 
  0.592398

Additionally, since premat() returns an array; either convert it to static or convert the variable that receives the return value of premat (in precess.jl and baryvel.jl) to static?

Not sure what you refer to, the PR already makes the array returned by premat static: https://github.com/JuliaAstro/AstroLib.jl/pull/46/files#diff-3466cd565556cdaaf4ea10f869aa4961

TestSubjector commented 7 years ago

Not sure what you refer to, the PR already makes the array returned by premat static: https://github.com/JuliaAstro/AstroLib.jl/pull/46/files#diff-3466cd565556cdaaf4ea10f869aa4961

Ah, didn't notice that. I was referring to the same.

I think almost everything is covered by the PR then.

giordano commented 7 years ago

I think almost everything is covered by the PR then.

I don't expect to have covered everything, but almost everything is a good result :-) Thanks!

giordano commented 7 years ago

Holding back the PR because of https://github.com/JuliaArrays/StaticArrays.jl/issues/321 which makes nutate type-unstable on Julia master https://github.com/JuliaArrays/StaticArrays.jl/issues/322 https://github.com/JuliaLang/julia/issues/24286

Edit: all issues resolved.