Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bug: running optimize() multiple times produces different result compared to running it once #228

Closed
dkobak opened this issue Jan 27, 2023 · 3 comments · Fixed by #229
Closed
Labels
bug Something isn't working

Comments

@dkobak
Copy link
Contributor

dkobak commented Jan 27, 2023

Running optimize(n_iter=1) for 100 times should give the same result as optimize(n_iter=100), but it doesn't. Here is a reproducible example:

n = 100
np.random.seed(42)
X = np.random.randn(n, 2)

from openTSNE.affinity import PerplexityBasedNN
from openTSNE import TSNEEmbedding
from openTSNE.initialization import random as random_init

A = PerplexityBasedNN(X)
I = random_init(n, random_state=42)

E1 = TSNEEmbedding(I, A, random_state=42)
E2 = TSNEEmbedding(I, A, random_state=42)

E1.optimize(n_iter=100, inplace=True)

for i in range(100):
    E2.optimize(n_iter=1, inplace=True)
    
plt.figure(figsize=(4, 4), layout='constrained')
plt.scatter(E1[:,0], E1[:,1])
plt.scatter(E2[:,0], E2[:,1])

tmp-tsne-bug

Maybe it has something to do with how the gains are saved in between the optimize() calls?

@dkobak
Copy link
Contributor Author

dkobak commented Jan 27, 2023

Hmm, does not look like it's due to gains: if I use 2 iterations instead of 100 in the snippet above, then E1 and E2 are still not identical, yet E1.optimizer.gains and E2.optimizer.gains are the same.

@dkobak
Copy link
Contributor Author

dkobak commented Jan 30, 2023

So I figured out what is going on here. update vectors are not save between the optimize() calls, so momentum term has no effect if one uses optimize(n_iter=1). I guess it's debatable what is better, but I would say that if we keep the gains between the optimize() calls, then we should also keep the update vectors.

I prepared a quick PR that implements that but unfortunately it makes some tests fail, and I could not fix it yet.

@pavlin-policar
Copy link
Owner

Thanks for tracking this down. This is definitely a bug. Calling optimize once with iter=100 should definitely be the same as calling optimize 100 times with iter=1.

@pavlin-policar pavlin-policar added the bug Something isn't working label Feb 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants