[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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



Hi Luc,

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.

Except these cases, I don't see any problem in using your new implementation. 

We'll give you some feedback asap.

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