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