Updated by Alexander Shishkov about 13 years ago
CvEM::predict
[[CvEM]]::predict and CvEM::calcLikelihood [[CvEM]]::calcLikelihood have one and the same bug starting at least from release version 2.3.0 and up to the current SVN (rev. 6603). It's merely a typo: somebody tried to account for 2Pi^(dims/2) factor and wrote "cvAndS" instead of "cvAddS" )).
The attached patch corrects this bug and also contains several other improvements of these functions:
1. The log-sum-exp trick can be done in place, without allocation of additional memory and copying of arrays.
2. Correcting for 2Pi^(dims/2) in CvEM::predict [[CvEM]]::predict is absolutely redundant because of the normalization which comes just several lines afterwards and thus can be avoided for performance reasons. Whereas "-0.5" scaling is still necessary and must be performed only if _probs != NULL (for the same performance reasons).
3. The log-sum-exp trick is surely also useful in CvEM::predict [[CvEM]]::predict (and even more than in calcLikelihood), thus it is implemented (in the same way as in CvEM::calcLikelihood, [[CvEM]]::calcLikelihood, with minor changes).
4. If CvEM::run_em [[CvEM]]::run_em runs with "start_step == START_E_STEP" and detects singularity of covariance matrix eigenvalues it just +silently+ exits, leaving "inv_eigen_values" uninitialized and "log_weight_div_det" with wrong values. IMHO it is not a good idea because it is done silently and if one will try to use "predict" after that she/he will wonder about results. It seems to me that a much better solution would be to just let it running, i.e. use pseudoinverce on the first step. After covariances will be recomputed on the m-step they will be almost sure nonsingular so we can proceed as usual and hopefully get meaningful results.
The attached patch has to be applied after my previous patch to rev. 6603 (regarding broken k-means).
With hope that this helps and with best regards,
Alexander Alekseychuk
[[CvEM]]::predict and CvEM::calcLikelihood [[CvEM]]::calcLikelihood have one and the same bug starting at least from release version 2.3.0 and up to the current SVN (rev. 6603). It's merely a typo: somebody tried to account for 2Pi^(dims/2) factor and wrote "cvAndS" instead of "cvAddS" )).
The attached patch corrects this bug and also contains several other improvements of these functions:
1. The log-sum-exp trick can be done in place, without allocation of additional memory and copying of arrays.
2. Correcting for 2Pi^(dims/2) in CvEM::predict [[CvEM]]::predict is absolutely redundant because of the normalization which comes just several lines afterwards and thus can be avoided for performance reasons. Whereas "-0.5" scaling is still necessary and must be performed only if _probs != NULL (for the same performance reasons).
3. The log-sum-exp trick is surely also useful in CvEM::predict [[CvEM]]::predict (and even more than in calcLikelihood), thus it is implemented (in the same way as in CvEM::calcLikelihood, [[CvEM]]::calcLikelihood, with minor changes).
4. If CvEM::run_em [[CvEM]]::run_em runs with "start_step == START_E_STEP" and detects singularity of covariance matrix eigenvalues it just +silently+ exits, leaving "inv_eigen_values" uninitialized and "log_weight_div_det" with wrong values. IMHO it is not a good idea because it is done silently and if one will try to use "predict" after that she/he will wonder about results. It seems to me that a much better solution would be to just let it running, i.e. use pseudoinverce on the first step. After covariances will be recomputed on the m-step they will be almost sure nonsingular so we can proceed as usual and hopefully get meaningful results.
The attached patch has to be applied after my previous patch to rev. 6603 (regarding broken k-means).
With hope that this helps and with best regards,
Alexander Alekseychuk