Free cookie consent management tool by TermsFeed Policy Generator

Changes between Initial Version and Version 1 of Ticket #1913, comment 22


Ignore:
Timestamp:
11/28/12 17:34:15 (11 years ago)
Author:
gkronber
Comment:

Legend:

Unmodified
Added
Removed
Modified
  • Ticket #1913, comment 22

    initial v1  
    11Review comments:
     2 * The changes in `NearestNeighbourModel` are OK
    23 * should we move the class `Matrix` into one of the base plugins (e.g., HeuristicLab.Common)?
    3  * tbc
     4 * In `NcaModel` line 92: shouldn't we use a matrix-multiplication here?
     5 * in `NcaAlgorithm` line 221: why do we not set a value when normalization <= 0.
     6 * Why does the algorithm use sparse matrix? Are the majority of the values of probabilities = 0?
     7 * I reviewed the `Gradient()` method (r8681) but have to admit that I'm not familiar enough with NCA to be sure that it is implemented correctly.
     8
     9Overall, the implementation looks good. However, the algorithm is hard to understand even when reading the referenced paper. Therefore, it would be great if the implementation is tested using unit test cases where the results of our implementation (esp. of the `Gradient()` method) are compared with the results of a reference implementation (e.g., from [http://www.cs.cmu.edu/~liuy/distlearn.htm Toolkit for Distance Metric Learning]  or from [http://homepage.tudelft.nl/19j49/Matlab_Toolbox_for_Dimensionality_Reduction.html Dimensionality reduction toolbox])?