kekeblom/DeepCGP

mistake in _cluster_patches

JanSochman opened this issue · 3 comments

Hi, while looking at the code I have found that in _cluster_patches in kernels.py, the array 'patches' is allocated too small -- (M x patch_length) instead of (M*samples_per_inducing_point x patch_length) -- and then the following loop preserves only the first M samples instead of all of them (the indexing is done in such a way that unfortunately no index out of bound error is emitted). This leads to a commonly displayed error that not enough cluster centers were found. Obviously, also the clusters are not initialized correctly and who knows what is the consequence for the training...

Anyway, a great paper and a piece of code! Thanks a lot for sharing it!

Good call. Just pushed a fix 2247aa6 for this issue. Thanks for reporting the issue!

It seems on mnist this warning still sometimes appears when using a lot of inducing points (e.g. 384) even when a lot more patches than inducing points are sampled.

Btw, why 384? I haven't been able to find any formula which would give me this number. Is there any reason for this particular value?

It's a multiple of 32 which is the amount of threads on one cuda warp. The computational burden will increase as the number of inducing points increase so it's a trade-off between cost and accuracy.

You can set it to whatever value you want.