-
Notifications
You must be signed in to change notification settings - Fork 163
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
Test failure on i386 #247
Comments
Oh it also fails on Ubuntu ppc64el. Here are the logs: |
That is interesting. Unfortunately, I don't know how I could debug this, as I do not have access to these platforms. Are there actually any visual differences when looking at these embeddings? I know this is a test run with no visuals, but have you tried looking at the difference between passing in a precomputed matrix and or letting openTSNE take care of that? |
Hello, I have uploaded the 1.0.0 version in Debian and yes the problem is still present. https://buildd.debian.org/status/logs.php?pkg=opentsne&ver=1.0.0-1&suite=sid This is really strange arm 32bit arch are passing the tests. This seems really i386 related... There is specific code for x86 cpu ? I removed the notiv flag during the compilation since this is a violation of the Debian policy. on i386 we do not support SSE instructions. https://wiki.debian.org/ArchitectureSpecificsMemo#i386-1 Cheers |
This could potentially have something to do with the Annoy library. I believe it has some AVX instructions and such, howver, I really don't know this code at all and wouldn't even know where to begin trying to fix it. However, it seems that maybe they have taken some care to add i386: https://github.com/spotify/annoy/pull/207/files So maybe it's not that? I generally tried to copy their setup pipeline into https://github.com/pavlin-policar/openTSNE/blob/master/setup.py as closely as possible, but perhaps I've messed something up? As I've said, I have no way to reproduce this and I am not familiar with the platform-specific compiler directives, so any help on this would be greatly appreciated. |
Just to clarify: is this the ONLY test that fails? I don't think this rest relies on Annoy: it uses the Iris data which is so small that openTSNE should use exact NN. |
Sorry, I totally missed the context. Only one test is failing! I have no idea what could be causing this. Could it be due to accumulating rounding errors or something? The actual embedding coordinates do seem to be ranked similarly. Or could it be that scipy computes distances a bit differently to scikit-learn on i386? |
No idea, but it does seem like the problem may be upstream. Would be interesting to compare sklearn NearestNeighbors results on X vs on pdist(X) with metric=precomputed, to see if there is indeed any difference. Is there any way to run this on i386 in a virtual machine? Otherwise, @utkarsh2102 @picca do you want to help debugging this? |
What about adding this test in the unit test suite, then I will be able to make it run on all our architectures.
We can use multiple release cycle in order to check this.
I do not have lot's of time myself for this, but, by packaging a OpenTSNE which contain this test is something possible for me.
for i386, maybe you can use qemu ?
|
@picca I was just wondering if you could run the following snippet in your Python environment on i386: import numpy as np
from sklearn.datasets import load_iris
from sklearn.neighbors import NearestNeighbors
from scipy.spatial.distance import pdist, squareform
X = load_iris()['data']
np.random.seed(42)
X += np.random.randn(*X.shape) / 10
nn = NearestNeighbors(metric="euclidean").fit(X)
distances1, indices1 = nn.kneighbors(n_neighbors=90)
nn = NearestNeighbors(metric="precomputed").fit(squareform(pdist(X)))
distances2, indices2 = nn.kneighbors(n_neighbors=90)
assert(np.allclose(distances1, distances2))
assert(np.all(indices1 == indices2)) In my environment this passes. |
Hey, thanks for working on this, @pavlin-policar. However, when building the package in i386, one of the tests fails:
The text was updated successfully, but these errors were encountered: