Ephemeris Data Rework
The way data for points and axes taken from ephemeris files is handled should be reworked. At the current status, the ephemeris provider is embedded into the frame system and the dedicated add_point_ephemeris! and add_axes_ephemeris! functions automatically look into that specific provider.
This system is however not very flexible, especially when considering other type of points\axes whose data may be taken from similar provider structures, e.g., IPF files or points whose data has been integrated.
The proposal is to remove from the frame system the field storing the ephemeris provider and instead pass it as an additional argument to the dedicated add_pont! and add_axes! functions.
I was wondering, is there a way to avoid completely having a cache within the FrameSystem for points/axes? Or better should we investigate this?
I think this has several advantages:
- No need for caching, with all types of issues that this might require. This is a duplication already, as, for example, ephemeris packages already have their own cache with the proper type management.
- That would also work for fixed translation-rotation -> we use the same interface as for the others, just return time-independent fixed vector/rotation.
- Simplify the overall interface (the functions do not depend on t and caches, but only on t).
@MicheleCeresoli I can have a look at this in the next few days!
I don't think it is that simple nor convenient in some ways:
- No need for caching, with all types of issues that this might require. This is a duplication already, as, for example, ephemeris packages already have their own cache with the proper type management.
The cache in Ephemerides.jl has a completely different purpose than the one we have implemented here. In that case what we store in the cache are the polynomial coefficients or other data that is read from the binary files plus other buffers that are requested to avoid allocations when evaluating the required states. Storing the coefficients avoids having to read and parse the binary data when the requested information belongs to the same logical record. Instead, the buffers that are stored in the caches for the polynomial evaluations depend directly on the input time, both in terms of type and value, so they aren't really caching anything, even if I request the same state at the same time consecutively.
On the other hand, the cache that is implemented here does exactly that. Removing it would definitely imply a significant slow-down every time we wish to interpolate ephemeris data. Imagine you are requesting the state of the Earth and Sun with respect to the Moon. According to the structure of the DEXXX kernels, the operations the Frame system does are the following:
-
Compute Earth position wrt to EMB
-
Compute EMB position wrt to SSB
-
Compute Moon position wrt to EMB
-
Compute EMB position wrt to SSB
-
Compute Earth-Moon
-
Compute Sun position wrt to SSB
-
Compute EMB position wrt to SSB
-
Compute Moon position wrt to EMB
-
Compute EMB position wrt to SSB
-
Compute Sun-Moon
If we do not cache the points positions, you can see that we would have a number of repeated operations: 4x the EMB-SSB computation and 2x the Moon-EMB computation.
- That would also work for fixed translation-rotation -> we use the same interface as for the others, just return time-independent fixed vector/rotation.
Although it could work for fixed translations\rotations (despite it would slow down the retrieval operations due to the FunctionWrappers overhead), it would not work for updatable points. Indeed, for this type of points you still need a place where you can store the values of your states
- Simplify the overall interface (the functions do not depend on t and caches, but only on t).
For points, removing the in-place operation filling the cache vector, would require that all functions return static vectors. This would first require changing the ephemeris interfaces. Removing the in-place operation would also remove the capability to support the usage of CALCEPH without allocations.
For axes, the interface would remain the same with the dependency on t, v1 and v2, where v1 and v2 are the vectors that are used for two-vector/computable axes.
This discussion should eventually be moved to a dedicated issue.
On the other hand, the cache that is implemented here does exactly that. Removing it would definitely imply a significant slow-down every time we wish to interpolate ephemeris data. Imagine you are requesting the state of the Earth and Sun with respect to the Moon. According to the structure of the DEXXX kernels, the operations the Frame system does are the following
Ok, I remember that was happening in the different ephemeris packages for some reason. Then we can keep the cache, nothing against it!
The idea was to see if there was a way to avoid this as it requires some additional effort to extend compatibility with other AD packages, while working out-of-place would still require this effort but on the lower-level packages only, while now we do that twice.
Would it be the same also for axes?
Although it could work for fixed translations\rotations (despite it would slow down the retrieval operations due to the FunctionWrappers overhead), it would not work for updatable points. Indeed, for this type of points you still need a place where you can store the values of your states
For axes, the interface would remain the same with the dependency on t, v1 and v2, where v1 and v2 are the vectors that are used for two-vector/computable axes.
For these, I wouldn't care much, as they could be easily added within the frame system with a custom object or using the frame system itself.
I would actually propose to remove them in any case and then add some specific "factories" using the new approach.
In the end, the objective here was mainly to simplify the way the package works, and make it more flexible :)
The idea was to see if there was a way to avoid this as it requires some additional effort to extend compatibility with other AD packages, while working out-of-place would still require this effort but on the lower-level packages only, while now we do that twice.
Mmh, the issue is not only related to the caching system. We still have to make the input\output types compatibles with those expected by the FunctionWrappers definitions. So, even without an internal cache, I believe that if you were to support multiple AD you would still need to manually specify all the inputs\output types in this package (the only workaround for FunctionWrappers might be having to accept allocations).
Would it be the same also for axes?
Yes, but axes are not really an issue since the ones defined in the .bpc do not require many concatenations: for example, both ITRF93 and PA440/420 are directly defined wrt the ICRF.
I would actually propose to remove them in any case and then add some specific "factories" using the new approach.
That could be a possibility yes.
For the next release, I would not change this caching system. The biggest drawback is at the moment associated with the Ephemeris data re-computations. A way forward could be to cache those information inside the structures defined in Ephemerides.jl, but it requires some investigations... but then again, this would only remove the cache whereas type-flexibility would still be limited by FunctionWrappers.
Yes, agreed, we can keep the caches. I would propose to re-organize them in a more structured object, such as:
struct FrameSystemNodeCache{T, C1, C2}
epochs::DiffCache{Vector{T}, Vector{T}}
cache::DiffCache{Vector{C1}, Vector{C2}}
nzo::Vector{Vector{Int}}
end
Then, review the rest as:
Axes
# Current structure
struct FrameAxesNode{O,T,N}
name::Symbol
class::Symbol
id::Int
parentid::Int
comp::ComputableAxesProperties
R::Vector{Rotation{O, T}}
epochs::Vector{T}
nzo::Vector{Int}
f::FrameAxesFunctions{T,O,N}
angles::Vector{DiffCache{MVector{N,T}, Vector{T}}}
end
# Proposal
struct FrameAxesNode{O, T}
name::Symbol
class::Int
id::Int
parentid::Int
f::FrameAxesFunctions{O, T}
# cache
cache::FrameSystemNodeCache{T, Rotation{O, T}}
end
Points
# Current structure
struct FramePointNode{O,T,N} <: AbstractJSMDGraphNode
name::Symbol
class::Symbol
axesid::Int
parentid::Int
NAIFId::Int
stv::Vector{DiffCache{MVector{N,T}, Vector{T}}}
epochs::DiffCache{Vector{T}, Vector{T}}
nzo::Vector{MVector{2, Int}}
f::FramePointFunctions{T,O,N}
end
# Proposal
struct FramePointNode{O, T} <: AbstractJSMDGraphNode
name::Symbol
class::Int
id::Int
parentid::Int
axesid::Int
f::FramePointFunctions{O, T}
# cache
cache::FrameSystemNodeCache{T, Translation{O, T}}
end
Where I already have a Translation type implemented as:
struct Translation{S, N <: Number} <: AbstractArray{N, 1}
v::NTuple{S, SVector{3, N}}
end
I performed performance tests already on it for both compositions with Rotations as well as other operation. No notable difference found (within ~ 0.2 ns difference ).
Graph
# Current
struct FrameSystem{O,T<:Number,S<:AbstractTimeScale,E<:AbstractEphemerisProvider,N}
eph::E
prop::FrameSystemProperties
points::MappedNodeGraph{FramePointNode{O,T,N},SimpleGraph{Int}}
axes::MappedNodeGraph{FrameAxesNode{O,T,N},SimpleGraph{Int}}
end
# Proposed
const FramesPointsGraph{O, T} = MappedNodeGraph{FramePointNode{O, T}, SimpleGraph{Int}}
const FramesAxesGraph{O, T} = MappedNodeGraph{FrameAxesNode{O, T}, SimpleGraph{Int}}
struct FrameSystem{O, T<:Number, S<:AbstractTimeScale}
points::FramesPointsGraph{O, T}
axes::FramesAxesGraph{O, T}
end
Done with #54 . Closing.