Summary:
This paper describes the design and implementation of a group of classes representing Statistical Distributions. The classes are intended to fill a void in the existing ITK Statistics Framework.

Open Science:
The paper fully adheres to the principles and practices of Open Science. It provides a clear description of the problem, it provides the source code of the implementation, as well as tests that verify the correct behavior of the implementation. The material is suficient for any reader to replicate with minimal effort the work described by the author.


Reproducibility:
This reviewer downloaded the source code, made a code review of it, and build the code and its tests. The code was built without any problem under cygwing with gcc 3.4. No modifications were needed in the code or the CMake configuration files. All the test passed without any problem.

A plot of the distributions would have been a nice addendum to the tests, but not totally necessary given that the distributions were already sampled in multiple points during the testing phase. The test verify the precision of the computation, which provides a lot of confidence on the correct implementation of the computation.

Use of Open Source Software:
The author based his implementation on the existing Statistical Framework of the Insight Toolkit. The source code of his contribution is also available in open source and it is intended to be included in ITK.

The final computation of some distributions is using the netlib library. The author did a very nice job on analyzing the issues that this library have with multi-threading, due to the fact that the library was translated from Fortran into C. This section of the paper should be highlighted, and in fact, given its relevance for the rest of the Insight Toolkit, it could probably be a paper in itself. It was particularly interesting the discussion about the computation of mathematicl precision on every platform.




Open Source Contributions:
The author provides the source code of his contribution. It already follows most of the coding style of the Insight Toolkit.

The code is usable without any changes. Although the reviewer has some reservations on the naming of methods (see below in Coding Style).
The paper presents a clear and didactic description of how the classes are intended to be used.

This reviewer was able to use the code in a matter of minutes (almost seconds).


Code Quality:

Coding Style:
The Methods have acronyms. This contradicts the standards coding guidelines.
Methods such as EvaluatePDF() should be named EvaluateProbabilityDensityFunction().
The code seems to be portable although this reviewer only tried it on Cygwin with gcc 3.4, on a single processor machine.

Applicability to other problems:
This work is quite important since it fills a fundamental void in the Statistical Framework of the Insight Toolkit. Given than these classes provide basic functionalities for computing statistical distributions, they will certainly be used all accross the toolkit, and in many medical image applications.

Suggestions for future work:
The Distributions provided by the author are intended for single value distributions. However, the rest of the Statistical framework in ITK is based on the concept of multi-variate statistics. It seems to be convenient to create variationos of these distributions that are applicable to single-value and to multi-valued situations. This is already proposed by the author in the Doxygen documentation of the classes.

Requests for additional information from authors:
[Did you find that information was missing from the paper? Maybe parameters for running the tests? Maybe some images were missing? Would you like to get more details on how the diagrams, or plots were generated?]

Additional Comments:

A) The following statement in the Doxygen documentation of the ProbabilityDistribution is unclear:

" ProbabilityDistribution also defines an abstract interface for setting parameters of distribution (mean/variance for a Gaussian, degrees of freedom for Student-t, etc.). "

It leads to believe that the ProbabilityDistribution is offering the union of the methods available in the subclasses. Which is not the case.


B) It is questionable whether the methods in the base class : ProbabilityDistribution should return ad-hoc default values, or instead they should be abstract. For example:

* HasMean
* HasVariance
* GetMean
* GetVariance


C) Operator assignement and Copy constructor declarations are missing from all the ProbabilityDistribution class. They should be declared private, and should not be implemented. This is part of the API of any class using SmartPointers.


D) It is questionable whether the parametric distributions should have the parameters to be member variables. The approach of requiring to pass the parameters along with the X value to be computed do not allow to use a generic ProbabilityDensity function and use polymorphism. In the current implementation, this would only be possible by using the "cannonical" parameters. Since polymorphism is not available in this current implementation, there is no use either for making these classes derive from the itk::Object and to use SmartPointers. Without polymorphism there is not much advantage in using this classes via pointers, instead they should simply be created as Functors that can be fully inlined at compilation time.

E) The naming of the functions "PDF()" and "EvaluatePDF()" appears to be redundant and confusing. A better naming could be to replace PDF() with

EvaluateCannonicalProbabilityDensityFunction()

or

EvaluateNormalizedProbabilityDensithyFunction()

while EvaluatePDF() should be named:

EvaluateProbabilityDensityFunction()

In any case it seems that this class is providing services with two different paradigms. In one, it is being used as a mathematical calculator, when the static methods are used. On the other, it is used as a modular component that would be plugged into a framework. It seems that those two roles should be provided by two independent classes, and that the second role could actually delegate the computation to the first role.


F) The constructor of the TDistribution declare a number of constants. They could take advantage of the "const" keyword in order to clarify their intent.

Summary:
Presents a class hierarchy for and instances of probability density functions for ITK. Each density function (e.g., chi-squared, Gaussian) provides PDF, CDF, and inverse-CDF member functions as well as static instances of those member functions.

The paper covers a wide variety of ways in which density functions (i.e., PDFs, CDFs, and inverse-CDF) can be efficiently used in medical image analysis and statistical testing.

The code is well written. Providing static methods, in addition to function methods, is outstanding.

Use of Open Source Software:
Good - but not excellent - would be improved by building upon the existing code in ITK and integrating with the classification hierarchy of itk - see comments below.


Open Source Contributions:
All code in the submission is available as open-source and is available for free. It is unlikely that this code infringes on patents or copyrights.

The authors also developed a modification to f2c that makes it produce thread-safe code for the netlib code included with this submission. The modification should also allow other f2c applications to produce thread-safe code. It is not clear, however, if that modification is going to be released open-source.


Code Quality:
It is not clear why a new class hierarchy for density functions is needed in ITK. In the Code/Numerics/Statistics directory there is a class hierarchy that provides some of the proposed functionality (itkDensityFunction is the existing base class and itkGaussianDensityFunction is a derived class). The code in this submission would greatly extend the functionality of the existing ITK classes, but it should extend and not duplicate. Duplication does not help the open-source community.

The code in the submission also does not duplicate all of the capabilities in the current ITK classes - the code in the submission cannot be used in the existing ITK classifier hierarchy (it is necessary to derive density functions used in the classifier from itkMembershipFunction).

Should an additional class hierarchy layer be added to distinguish parametric from non-parametric density functions?
For parametric density functions, the code in the submission should also contain Get/SetParameter methods - to support their use in itk's existing mixture modeling hierarchy.

GetMean and Get Variance should throw exceptions when called for density functions for which they have no meaning - returning NAN is not consistent with ITK style - itk style is to throw exceptions to indicate errors - not to return illegal values that must be tested to detect the error.

Other small issues - preferred, not required:
itkTDistribution should perhaps be renamed itkStudentsTDistribution.

Why is there a SetMean for Gaussian and not a SetMean for Student's T?


Applicability to other problems:
Good - but again, if these classes built upon existing ITK standards, then their applicability would be greater.

I was able to compile the code, run the tests. I haven't much to say about the code, cause the class hierarchy and the code is great.

There are multiple ways to generate normal random variates now. For instance
itk::Statistics::NormalVariateGenerator, itk::MersenneTwisterRandomVariateGenerator (Box-muller method) and the GaussianDistribution can all generate Gaussian variates, although the last one uses the MersenneTwister to convert a uniform distribution to a normal distribution. Each of them seems to use different methods though and it would be nice to know which one is the fastest.