Re: [Orekit Developers] Shouldn't Transform implement TimeStamped and TimeShiftable

[Luc Maisonobe <Luc.Maisonobe@c-s.fr>]

Le 12/06/2012 16:11, Tanguy Yannick a écrit :
> Hi Luc,

Hi Yannick,

> 
> I globally agree with your arguments : in most cases, it make sense to link a Transform with a date. 
> But how do yo deal with static transformations, like Transform.IDENTITY, or like translations ? What date do you use ? 
> In some of our code, we use "Transform" to link different part of a vehicule (ex : the main box and its solar arrays). Most of the time, theses transformations are not time-dependent.

In these cases, I use a dummy date. If you look at Transform.IDENTITY,
you will see the one I selected is AbsoluteDate.J2000_EPOCH.


> 
> Except these cases, I don't see any problem in using your new implementation. 
> 
> We'll give you some feedback asap.

thansk a lot

Luc

> 
> Best regards,
> 
> Yannick
> 
> 
> Le 11/06/2012 12:37, MAISONOBE Luc a écrit :
>>> Hi all,
>>>
>>> I am currently working on the multi-threading issues, focusing on Frames
>>> as a first step forward. As I wanted to use the new TimeStampedCache to
>>> hold transforms between a frame and its parent frame, I considered
>>> changing Transform to directly implement TimeStamped. Of course, there
>>> are pros and cons about this change.
>>>
>>> The pros are:
>>>
>>>  - from a purely flight dynamics point of view, it does make sense.
>>>    Transforms are specifically used to glue frames with each other,
>>>    either directly for a frame with respect to its parent frame or
>>>    indirectly by building a combined transform for converting coordinates
>>>    between two frames which are not direct parent. Most of these transforms
>>>    are valid only at a specific date, and only a few of them are
>>> independent
>>>    of date.
>>>
>>>  - whenever we use a transform, we either already have a date available
>>>    or the transform is date independent, so it is really easy to link the
>>>    transform with the date. I did it, and it took me less than one hour to
>>>    propagate it throughtout the library, despite there are many many use
>>>    of transforms (eclipse found 340 references to Transform)
>>>
>>>  - I suppose this would simplify some user code, as for now users need to
>>>    hold the date and the transform together by themselves, typically by
>>>    having two parameters to any function that needs a transform, just to
>>>    know the associated date
>>>
>>>  - of course, if Transform implements TimeStamped, it is much simpler to
>>>    use it with TimeStampedCache (otherwise, we would need to add an
>>>    intermediate class to pack the date and the transform together)
>>>
>>>  - if Transform implements TimeStamped, it would be easy to have it also
>>>    implement TimeShiftable and have a new possibility to compute simply
>>>    and very fast small time shifts in frames transforms without recomputing
>>>    every individual transform throughout the frames tree (this is used for
>>>    example to implement the shift method in the Attitude class, so the code
>>>    which exist in the Attitude class could be moved to the Transform class)
>>>
>>> The cons are:
>>>
>>>  - It is a backward incompatible change (so if we want to do this, we have
>>>    to do this now as 6.0 is a major release where incompatible changes are
>>>    allowed, which will not be possible with later 6.1 minor release)
>>>
>>>  - All Transform constructors must be changed to add the date (this is in
>>>    fact quite simple, as it introduce compilation errors that IDE like
>>>    Eclipse spot immediately so it is guaranteed no call will be missed)
>>>
>>>  - The object holds one more field, so it is larger. I'm not sure this is
>>>    really a problem, as Transforms are often transient objects which are
>>> not
>>>    used or serialized in large numbers
>>>
>>> I have made the change to check if everything was simple, and indeed it
>>> was. One unexpected change however was about combining two Transform
>>> instances to build a new one. The current constructor takes two arguments:
>>>
>>>   public Transform(Transform first, final Transform second) {
>>>     ...
>>>   }
>>>
>>> I have decided to add a date parameter also for this constructor, and to
>>> *ignore* the dates of the two raw transform. The rationale behind this
>>> choice was that sometimes we do combine transforms built at different
>>> times. A typical example is when doing some coordinates conversion
>>> involving one frame which is fixed with respect to its parent like
>>> EME2000 with respect to GCRF or a topocentric frame with respect to the
>>> body frame or some sensor frame with respect to the spacecraft body
>>> frame. The fixed transform is built as an arbitrary constant date (for
>>> example AbsoluteDate.J2000_EPOCH) and never changed. When this transform
>>> is combined with the next transform in the tree, the other transform
>>> depends on date and will not be AbsoluteDate.J2000_EPOCH. Another use
>>> case is when combining transform for interpolating, such computation
>>> involves combining the transform at date t + h with the inverse of the
>>> transform at date t to evaluate the evolution between the two dates.
>>> Both cases show combining transforms at different dates should be
>>> allowed without errors. It seems impossible to guess automatically which
>>> date to use. So I added the parameter and let the caller specify the
>>> date of the new transform, which may or may not be one of the underlying
>>> transforms date.
>>>
>>> I have not committed the change yet. What do you think about it ?
>>
>> As nobody complained, the changes have been committed in the Git
>> repository (see
>> <https://www.orekit.org/forge/projects/orekit/repository/revisions/69dce75ee388b85278c89a83df20dc191509fb34>>).
> 
> 
>> If interested persons could review this change, I'd be happy to have
>> some feedback.
>>>
>>> Luc
>>>
>>> ----------------------------------------------------------------
>>> This message was sent using IMP, the Internet Messaging Program.
>>>
>