Quality Control

Jump to: navigation, search

This page is dedicated for planning and undertaking tasks related to Quality Control.

Bug Tracker

Bug Tracker Summary


Tracker Refactoring Comments

State Machines

Transition Table Completeness

State Machine Transition Tables Completeness

Ongoing Code Review

igstktrackerport.cxx (19 June 2007)

Fix the following code:

TrackerTool * TrackerPort ::GetTool( unsigned int toolNumber ) {
  if( toolNumber >= m_Tools.size() )
  return m_Tools[toolNumber];

Comment: The only way that I can see for the state machine to remove this "if", is for there to be a different state for every possible vector size.Dgobbi 15:36, 22 June 2007 (EDT)

igstkTracker (19 June 2007)

validity time

The user must be able to set the validity time of the acquired transformations. This should probably be done in the igstkTracker class using a SetValidityTime(unsigned long) method. It cannot be hard coded as above a certain threshold the user will experience dangerous behavior such as a transform is still valid long after the user knows the setup has changed. More importantly because of the buffered approach used in IGSTK the program may be aware that currently the tool is not visible but the previous transform is still valid timewise so it will continue to report that transfrom. To get a feeling for this just change the validityTime to 1000 and quickly move a tool out of the work volume. In any case the validityTime is currently hard coded for each of the trackers which is bad practice:

  • aurora:

const TimePeriodType validityTime = 400.0;

  • polaris:

const TimePeriodType validityTime = 400.0;

  • flock of birds:

const TimePeriodType validityTime = 100.0;

Comment: Done and committed, the Tracker class now has a method for setting the ValidityTime. The derived classes use this value instead of their own hard-coded values. Dgobbi 17:32, 22 June 2007 (EDT)

igstkLandmark3DRegistration (18 May 2007)

  1. The class and method documentation does not state what events are generated.Comment: Documentation added to the event macro definitions Andinet 10:08, 27 June 2007 (EDT)
  2. The method ComputeRMSError:
    • Doesn't use the Request prefix.
    • Is not part of the state machine and will return invalid values if invoked before registration is performed, or when only image or tracker points have been set.
    • Should not exist on its own. The RMS should be computed as part of the registration process and the method should be RequestGetRMSError().
    • Returns a value contrary to the practice that igstk methods should be void with values returned as payloads of generated events. Comment: I agree with all your comments. I made the necessary fixes Andinet 10:46, 27 June 2007 (EDT)
  3. Not clear why the event TransformInitializerEvent is defined, other than to serve as a common superclass for all registration related event? In any case a more appropriate name is Landmark3DRegistrationEvent as igstk is supposed to hide the underlying itk transform initializer from the user.
  4. Events are defined using the itkEventMacro and not the igstkEventMacro (igstk just wraps itk, but either get rid of igstk macro or use it). Comment: Fixed. Andinet 09:47, 27 June 2007 (EDT)
  5. Adding the image and tracker points in separate methods seems dangerous, as the data to the algorithm is pairs of points, perhaps use std::pair? Comment: I don't think it is dangerous. Actually, it enforces a safe procedure to specify corresponding landmark coordinates. Furthermore, all the data might not be available pairwise upfront. Andinet 10:05, 27 June 2007 (EDT)
  6. Error events are not generated for the following scenarios, should generate InvalidRequestErrorEvent() (In all likelihood this is the situation across IGSTK due to the way the state machine is built):
    • Invoking RequestGetTransform() when the state machine is not in TransformComputed state does not generate any error event.
    • Invoking RequestAddImageLandmarkPoint, RequestAddTrackerLandmarkPoint when the state machine is in TransformComputed state doesn't add the points and doesn't generate an error event. Comment: These are all missing transitions. I have added them to the state machine Andinet 11:27, 27 June 2007 (EDT)

igstkTransform.cxx (27 June 2007)

  • Is there a reason why the class doesn't have methods for transforming itk points and vectors? The current situation requires that the user create a separate itk rigid transformation to perform these basic operations. This can be achieved if the igstkTransform is designed as a decorator to the itkVersor3DRigidTransform.
  • Address the comment found in the code or remove it.
::operator==( const Transform & inputTransform )
  // FIXME Discuss how the == operator should be defined for the Transform.
  // In the meantime, let's go safe and consider two transforms equal
  // only if they are the same object in memory. It is arguable whether this
  // should be a method based on how close the numerical content of the two
  // transforms is, based on some tolerance.
  return (this==&inputTransform);
  • Why is transform composition a class method (static) and not an object method?

current syntax:

  static Transform TransformCompose(Transform leftTransform,
                                    Transform rightTransform);
T3 = igstk::TransformCompose(T1,T2);

possible object method syntax:

Transform TransformCompose(Transform rightTransform);
T3 = T1.TransformCompose(T2);