pavlin-policar / openTSNE

Extensible, parallel implementations of t-SNE
https://opentsne.rtfd.io
BSD 3-Clause "New" or "Revised" License
1.42k stars 157 forks source link

Fix BH collapse #235

Closed pavlin-policar closed 1 year ago

pavlin-policar commented 1 year ago
Issue

Fixes #223.

The is_duplicate function in the BH implementation had a threshold set to 1e-6, likely taken from other implementations, perhaps scikit-learn. This meant that during the EE phase, where everything was squished together, this condition was triggered a lot, meaning BH wasn't splitting up the points into different quadrants, but was pretending it was dealing with a single point. This mean that there were no repulsive forces being applied to the points at all, leading to an increasing collapse of all the points.

Description of changes

I simply changed the threshold for duplicate detection to machine precision, and the issue goes away. I also copied over the test by @dkobak from #234, but I've also let it run the standard phase. We want to make sure that the embedding isn't collapsed at the end of the optimization, and we don't really care if it is super compressed at the end of the EE phase.

EDIT: Not machine precision, because then, duplicate detection actually doesn't work, and the tests fail on iris. Setting the threshold to 1e-16 resolves both issues.

Includes
dkobak commented 1 year ago

With all the recent changes it would be great to release a new version (0.6.3 or 0.7), what do you think? Some of the changes (like new learning rate defaults) affect the outcome with default params.

pavlin-policar commented 1 year ago

Yes, there have been a few changes since 0.6.2. We've added jittering which actually does change the resulting embeddings a bit, so this probably warrants an 0.7.0 release. The rest was mainly bugfixes I think.

I'd like to get #226 solved first, and there are some issues on conda, but once these get resolved, I'll make a new release.

dkobak commented 1 year ago

Yes, init jittering, but also learning rate that is now set to n/exag in each epoch, and momentum fixed to 0.8 throughout.

Cool.