[IGSTK-Developers] Conclusion on igstkTransform issues - David&Patrick

[David Gobbi]

Hi Luis, Patrick,

In general I don't like "special values" and would prefer if  there was a
separate flag in igstkTransform to identify static vs. dynamic.

This would mean adding methods like SetStaticTranslationAndRotation(),
and the "static" flag would be set when these methods are called.  The
SetStaticFlag() method for setting the flag would be private, and there 
would
be a public GetStaticFlag() method for getting the flag.  How does this 
sound?

On problem of defining a special value like "Forever" is that simple math on
very large floating point values can cause bad things to happen.  For 
example,
if "Forever" is used as a duration, then "Forever" plus the start time might
cause an overflow, or it might be equal to "Forever" due to limited 
numerical
precision, which is counter-intuitive.

So what I'm saying is that if we use a special value, we would need to check
for that special value whenever we do any math that involves the time.  But
if we're checking for a special value, we might as well just have a flag
instead.

 - David


Luis Ibanez wrote:
>
>
> Hi Patrick,
>
>
> You are right about the potential misuse of the long times.
>
>
> We can remomve that risk when we add a specific type for
> representing Time and TimeLapses, since that type could
> define a constant that is considered to be an infinite time.
>
>
> In practice we can take the NumericTraits::max() of the
> type that we choose for internal representation of the time.
> We could call that static value something like:
>
>
>                Time::Forever()
>
>
> In this way, the intent of the value will be clearer
> to the readers of the code.
>
>
>
>    Luis
>
>
> -----------------------
> Patrick Cheng wrote:
>> Hi Luis,
>>
>> I agree on statement 2) and 3).
>>
>> But for 1), to be 'safe by design', we should not make any 
>> presumption that an operation does no go over a "long period of time".
>>
>> This "long duration" is easily being misused, and at least, it's not 
>> consistent through out our code. some examples:
>>
>> **********************************************************************
>> [[[Super Long Period]]]
>> igstkTrackerTool.cxx
>> Line 31
>>   m_ToolCalibrationTransform.SetToIdentity( 1e300 );
>>
>> igstkMeshReader.cxx
>> line91-92:
>>   // Provide an identity transform with a long validity time.
>>   const double validityTimeInMilliseconds = 1e30;
>>
>> [[[Wrong]]]
>> igstkPivotCalibration.cxx
>> Line 149
>> this->m_CalibrationTransform.SetTranslationAndRotation( translation, 
>> quaternion, 0.1, 1000);
>> Line 286
>> this->m_CalibrationTransform.SetTranslation(translation, 0.1, 1000);
>>
>> igstkPrincipalAxisCalibration.cxx
>> Line 204
>> this->m_CalibrationTransform.SetRotation( quaternion, 0.1, 1000);
>> ************************************************************************
>>
>> Luis Ibanez wrote:
>>
>>>
>>> Hi Patrick,
>>>
>>>
>>> 1) The double use of Static and Dynamic transform will require
>>>    SpatialObject to have both of them as member variables.
>>>    Since SpatialObjects should be able to be "static" or "tracked".
>>>
>>>    What is the difficulty with having the "static" transforms be
>>>    represented with a time stamp of long duration ?
>>>
>>>
>>> 2) When composing transforms, the time stamp of the final transform
>>>    must by the intersection among all the time stamps of the transforms
>>>    involved on the composition.
>>>
>>>    We could add to the TimeStamp class, as service method that will
>>>    compote the intersection between two TimeStamps. This operation
>>>    is associative, so by applying it by pairs we can compose any
>>>    number of TimeStamps.
>>>
>>>
>>> 3) It is better to have a explicit "Compose" method, than having
>>>    an operator overload.  The Compose method name could also make
>>>    clearer whether we are pre-composing or post-composing the
>>>    transform.
>>>
>>>
>>>
>>>
>>>   Luis
>>>
>>>
>>>
>>> ---------------------
>>> Patrick Cheng wrote:
>>>
>>>> Hi everybody,
>>>>
>>>> This is the conclusion of the discussion between david and me. 
>>>> Welcome to comment on it.
>>>>
>>>> ======================================================================= 
>>>>
>>>>
>>>> Problem 1. We need both static and dynamic transform. (Registration 
>>>> and calibration transforms should be static, and tracker transforms 
>>>> should be dynamic)
>>>>
>>>> Solution: Make subclasses of igstk::Transform,
>>>> igstk::DynamicTransform and igstk::StaticTransform StaticTransform 
>>>> which do not have time stamp. In base class we provide pure virtual 
>>>> fuction:
>>>> bool IsStatic()/IsDynamic();
>>>>
>>>>   Q? Do we some times have to switch the transform of a object from 
>>>> dynamic to static or the other way around?
>>>>
>>>> ======================================================================= 
>>>>
>>>>
>>>> Problem 2. We currently don't have a simple transform compose 
>>>> method, and we are not taking care of time stamps in the current 
>>>> implementation of the transform multiplication.
>>>>
>>>> e.g.
>>>> Ta is static transform, Tb is dynamic, and Tc is dynamic.
>>>> When we do T = Ta * Tb * Tc
>>>> to get the final valid time stamp for T, we should first ignore Ta, 
>>>> and pick up the earlier expiration time from Tb and Tc, and minus 
>>>> current time, to get the valid time period for the T, and set a 
>>>> right time stamp for it.
>>>>
>>>> Solution: Add a simple function or operator such as:
>>>> transform = transform1.compose( transform2 )
>>>> equals to:
>>>> transform = transform1 * transform2
>>>>
>>>> This will avoid the wrong matrix composition, and make code simpler 
>>>> and cleaner.
>>>> Also this compose() method will calculate the correct time stamp 
>>>> automatically according to predefine rules.
>>>>
>>>> ======================================================================= 
>>>>
>>>>
>>>> David, I hope you still agree on these points.
>>>>
>>>>
>>>
>>>
>>>
>>
>>
>
> _______________________________________________
> IGSTK-Developers mailing list
> IGSTK-Developers at public.kitware.com
> http://public.kitware.com/cgi-bin/mailman/listinfo/igstk-developers
>