[IGSTK-Developers] Conclusion on igstkTransform issues - David&Patrick
David Gobbi
dgobbi at atamai.com
Tue Mar 7 16:44:00 EST 2006
Hi Luis,
I missed your point about defining special types for elapsed
vs. abolute times. If we do that, then I agree that we can redefine
mathematic operations to make sure that special "Forever" value
can be treated properly.
It will be necessary to make sure that there is never silent
conversion between these types and "double". I recommend using
"Time" and "TimeDuration" as the names of these types.
- David
Luis Ibanez wrote:
>
> Hi David,
>
> Adding boolean Flags inside classes is poor way of
> implementing the logic that should be managed by a
> State Machine.
>
> The flags will plague the code with "if" statements,
> and will generate spaghetti logic that cannot be fully
> tested nor formally verified.
>
> Conceptually, any new extra boolean flag in a class that
> already has a State Machine is equivalent to duplicating
> the number of States in that class, since now the developer
> has to consider the effect of the flag on every transition.
> Although it is likely that the flag will affect only a small
> subset of all the States, it still will be up to the developer
> to make sure that she/he will hack the appropriate places in
> the code. The process of maintaining such style of code over
> many years is very error-prone.
>
>
> I agree with you that the use of special values is not the
> correct way of managing error conditions, nor special cases.
> My point, that I probably exposed poorly in my previous email,
> is that by replacing the plain "doubles" with a custom made C++
> class for representing the concept of Time and TimeLapses is the
> best way of enforcing that the values will be used consistently
> and that the application developers will not have the option of
> performing risky operations.
>
>
> A custom made Time class may have its own algorithmic rules,
> such as making sure that:
>
> Forever + 1000 = Forever
>
> The point is that the Time class will remove from the API
> the notion that a number is used for representing time.
> When I mention the concept of "Forever", more than a value
> in the Time class, it is a "State" of the time class. The users
> of the Time class will not have to perform checks ("if"s) on
> its state, instead we will implement inside the Time and TimeLapse
> classes all the algebraic operations that we need for the time.
>
>
> If that state is managed by the Time class itself, we can make sure
> that Time operations are done consistently all over the toolkit,
> instead of relying on developers to use the rules correctly on
> every new class.
>
>
>
> Luis
>
>
>
>
> ------------------
> David Gobbi wrote:
>> 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
>>>
>>
>>
>>
>
>
More information about the IGSTK-Developers
mailing list