Closed prakharcode closed 6 years ago
Merging #23 into master will increase coverage by
0.01%
. The diff coverage is100%
.
@@ Coverage Diff @@
## master #23 +/- ##
==========================================
+ Coverage 98.64% 98.66% +0.01%
==========================================
Files 5 5
Lines 74 75 +1
==========================================
+ Hits 73 74 +1
Misses 1 1
Impacted Files | Coverage Δ | |
---|---|---|
src/AstroBase.jl | 98.3% <100%> (+0.02%) |
:arrow_up: |
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 2fe11dc...24aa3ee. Read the comment docs.
Which error?
there's a difference in ERFA and Julia's implementation. For any random matrix A and B and a random angle.
julia> celestial_to_terrestrial_matrix(a, 1.2, b)
3×3 Array{Float64,2}:
1.37602 -0.539649 1.37863
1.42513 -0.75699 1.38733
1.40573 -0.898893 1.21214
julia> ERFA.c2tcio(a, 1.2, b)
3×3 Array{Float64,2}:
1.46296 0.722696 0.929215
-0.165709 -0.16177 -0.32488
1.04642 0.574121 0.529976
Probably because eraRxr(A, B)
actually performs B * A
?
julia> A = collect(reshape(1.0:9.0, 3, 3))
3×3 Array{Float64,2}:
1.0 4.0 7.0
2.0 5.0 8.0
3.0 6.0 9.0
julia> B = collect(reshape(10.0:18.0, 3, 3))
3×3 Array{Float64,2}:
10.0 13.0 16.0
11.0 14.0 17.0
12.0 15.0 18.0
julia> A * B
3×3 Array{Float64,2}:
138.0 174.0 210.0
171.0 216.0 261.0
204.0 258.0 312.0
julia> B * A
3×3 Array{Float64,2}:
84.0 201.0 318.0
90.0 216.0 342.0
96.0 231.0 366.0
julia> ERFA.rxr(A, B)
3×3 Array{Float64,2}:
84.0 201.0 318.0
90.0 216.0 342.0
96.0 231.0 366.0
julia> ERFA.rxr(B, A)
3×3 Array{Float64,2}:
138.0 174.0 210.0
171.0 216.0 261.0
204.0 258.0 312.0
or, more probably, (A' * B')'
(= B*A
), given the different memory layout of matrices between C and Julia
I think that we do not really this function. Multiplying matrices in C is a pain and a simple function like this helps but in Julia it's a piece of cake...
Sure, I'll close it then after the implementations.
I will close this now so we have it off the to-do list.
@giordano and @helgee any idea why this error is coming up?