FrameTransformations.jl icon indicating copy to clipboard operation
FrameTransformations.jl copied to clipboard

Ephemeris Data Rework

Open MicheleCeresoli opened this issue 1 year ago • 5 comments

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.

MicheleCeresoli avatar Mar 06 '24 22:03 MicheleCeresoli

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!

andreapasquale94 avatar Mar 08 '24 19:03 andreapasquale94

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:

  1. Compute Earth position wrt to EMB

  2. Compute EMB position wrt to SSB

  3. Compute Moon position wrt to EMB

  4. Compute EMB position wrt to SSB

  5. Compute Earth-Moon

  6. Compute Sun position wrt to SSB

  7. Compute EMB position wrt to SSB

  8. Compute Moon position wrt to EMB

  9. Compute EMB position wrt to SSB

  10. 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.

MicheleCeresoli avatar Mar 09 '24 09:03 MicheleCeresoli

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 :)

andreapasquale94 avatar Mar 09 '24 15:03 andreapasquale94

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.

MicheleCeresoli avatar Mar 10 '24 11:03 MicheleCeresoli

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

andreapasquale94 avatar Mar 10 '24 14:03 andreapasquale94

Done with #54 . Closing.

andreapasquale94 avatar May 31 '24 10:05 andreapasquale94