[Insight-developers] Adding Consistency check to itk::BinaryFunctorImageFilter : Design Question

Luis Ibanez luis.ibanez at kitware.com
Thu Dec 3 21:13:17 EST 2009 

Brad,

I like your suggestion of having a specific method for this sort of
verification.

Our previous plan was to simply call the check methods "somewhere"
in BeforeThreadedGenerateData(),.. which certainly can become very
messy.

Your approach will be a lot more systematic.

Plus it has the advantage of providing a clear (an appropriate)
name for what the method is doing.


 Anyone has an objection to Brad suggestion ?


          Luis


------------------------------------------------------------------------------------------------
On Thu, Dec 3, 2009 at 10:30 AM, Bradley Lowekamp
<blowekamp at mail.nih.gov> wrote:
> Luis,
> This is very likely taking a step too far back, and I just had the thought
> it so it has not been fully flushed out.
> It seems there is a phase of the pipeline that is missing: to verify the
> inputs and outputs of a filter. In addition to what you have mentioned,
> there is also the task of verifying the correct number of inputs (did the
> user forget to set an input). Currently, this is done in
> itkProcessObject::UpdateOutputData, which burdens all the Requested region
> methods to check to see if there is actually an input. Some filters will
> segfault if the inputs are not correctly set during the
> PropagateRequestedRegion phase of the pipeline.  One possibility would be to
> add an additional method the pipeline to:
> ProcessObject::VerifyInputs()
> DataObject::UpdateOutputInformation()
> ProcessObject::VerifyInputInformation()
> DataObject::PropateRequestedRegion()
> DataObject::UpdateOutputData()
>
> This is likely a bad idea and the result of spending too much time looking
> at the PropagateRequestedRegion phase of the pipeline and wishing inputs
> could be verified before.
> However, the validation check proposed in the BeforeThreadedDataGenerated
> seemed a bit late to me.
> Brad
> On Dec 2, 2009, at 5:39 PM, Luis Ibanez wrote:
>
> Gaetan,
>
> That's a very good point.
>
> The method should indeed be available at a higher level
> in the filter hierarchy. Probably at the level of the
>
>                 itkImageToImageFilter
>
> How about offering a method that does:
>
>    bool CheckImageInformationConsistency(
>         const ImageBase * image1,
>         const ImageBase * image2
>         ) const;
>
> This could still use helper method for specific
> comparisons, such as
>
>    bool CheckImageSpacingConsistency(
>         const ImageBase * image1,
>         const ImageBase * image2
>         ) const;
>
> ...
>
> bool CheckImageOriginConsistency( ...)
> bool CheckImageDirectionConsistency(...)
>
> ....
>
>
> Looking at what we are comparing, this is all
> information that is available at the level of the
> ImageBase, therefore we may manage to base
> it only on the image dimension.
>
> Filters down the hierarchy could use these
> methods in different combinations, according
> to what applies to their particular cases.
>
> BTW: one concern is that the Origin, Spacing
> and Direction comparisons may have to be
> made with a certain numerical tolerance.
>
>
>
> What do you think ?
>
>
>       Thanks
>
>
>               Luis
>
>
>
> -----------------------------------------------------
> 2009/12/2 Gaëtan Lehmann <gaetan.lehmann at jouy.inra.fr>:
>
> Hi Luis,
>
> I personally prefer your first approach. Don't you think that
>
> CheckInputImagesConsistency() should be in BeforeThreadedGenerateData() in
>
> BinaryFunctorImageFilter directly, instead of its subclasses? The tests may
>
> be desactivated in the subclasses if needed, by reimplementing
>
> BeforeThreadedGenerateData().
>
> Also, this bug must also be in many other classes which are processing
>
> several inputs - maybe those methods should be added in a more basic class,
>
> so they can be reused in those filters as well?
>
> Gaëtan
>
>
> Le 2 déc. 09 à 21:12, Luis Ibanez a écrit :
>
> The Bug 9893:
>
>           http://www.itk.org/Bug/view.php?id=9893
>
> calls for adding a check of consistency between the two input
>
> images of filters that derive from itkBinaryFunctorImageFilter.
>
> The example, is the AddImageFilter, in which we implicitly
>
> assume that the two images passed as input have:
>
>   1) The same number of pixels along each dimension
>
>   2) The same origin
>
>   3) The same spacing
>
>   4) The same direction
>
>
> Currently there is no check for these assumption,
>
> and therefore, a user can pass two images of different
>
> sizes, and get a segmentation fault at run time.
>
> ---
>
> The proposed fix is to add the following methods:
>
>  /** These method verify if the two input images have the same
>
>  *  size, origin, spacing and orientation. It is intended to be
>
>  *  called from some derived filters, when they require the two
>
>  *  inputs to be consistent. The method throws and exception if
>
>  *  the two input images are not consistent. */
>
>  void CheckInputImagesConsistency();
>
>  void CheckInputImagesConsistencyInSize();
>
>  void CheckInputImagesConsistencyInOrigin();
>
>  void CheckInputImagesConsistencyInSpacing();
>
>  void CheckInputImagesConsistencyInDirection();
>
>
> to the protected section of the itkBinaryFunctorImageFilter.
>
>
> Then, most of its derived classes will call the method:
>
>           CheckInputImagesConsistency()
>
>
> (which internally will call all the other methods, each one
>
> of which could throw an exception if the consistency check
>
> fails).
>
>
> Particular filters that do not require all checks, could call
>
> only those that apply.
>
> The suggested place to make these calls is the method
>
>                BeforeThreadedGenerateData();
>
>
> ---
>
> An alternative approach is to set these calls in the
>
> itkBinaryFunctorImageFilter, and add a boolean that
>
> sets whether the check should be performed or not.
>
> Derived filters could then set that boolean in their
>
> constructors.
>
> ----
>
>
> Any preferences between the two approaches above ?
>
> Any objections ?
>
> or Backward Compatibility concerns ?
>
>
>    Thanks
>
>
>         Luis
>
> _______________________________________________
>
> Powered by www.kitware.com
>
> Visit other Kitware open-source projects at
>
> http://www.kitware.com/opensource/opensource.html
>
> Kitware offers ITK Training Courses, for more information visit:
>
> http://kitware.com/products/protraining.html
>
> Please keep messages on-topic and check the ITK FAQ at:
>
> http://www.itk.org/Wiki/ITK_FAQ
>
> Follow this link to subscribe/unsubscribe:
>
> http://www.itk.org/mailman/listinfo/insight-developers
>
> --
>
> Gaëtan Lehmann
>
> Biologie du Développement et de la Reproduction
>
> INRA de Jouy-en-Josas (France)
>
> tel: +33 1 34 65 29 66    fax: 01 34 65 29 09
>
> http://voxel.jouy.inra.fr  http://www.itk.org
>
> http://www.mandriva.org  http://www.bepo.fr
>
>
> _______________________________________________
> Powered by www.kitware.com
>
> Visit other Kitware open-source projects at
> http://www.kitware.com/opensource/opensource.html
>
> Kitware offers ITK Training Courses, for more information visit:
> http://kitware.com/products/protraining.html
>
> Please keep messages on-topic and check the ITK FAQ at:
> http://www.itk.org/Wiki/ITK_FAQ
>
> Follow this link to subscribe/unsubscribe:
> http://www.itk.org/mailman/listinfo/insight-developers
>
> ========================================================
>
> Bradley Lowekamp
>
> Lockheed Martin Contractor for
>
> Office of High Performance Computing and Communications
>
> National Library of Medicine
>
> blowekamp at mail.nih.gov
>
>

More information about the Insight-developers mailing list