k-nn edge filter about benchmarking-gnns HOT 8 CLOSED

Off the top of my head, we put the -1 there to not include the node itself in its own k-nn neighbors set. We do the same when building TSP graphs.

@amauriholanda, can you verify/demonstrate this bug via, e.g., this notebook: https://github.com/graphdeeplearning/benchmarking-gnns/blob/master/visualization/superpixels_visualization_mnist.ipynb

First, the self-loop produces 0, but what we want is the largest similarity value(0 is the minimum one among all values)

That's my opinion, what do you think about this?

from benchmarking-gnns.

Hi @amauriholanda, thank you for your interest in our work. Actually the original code in the file data/superpixels.py does not miss the nearest neighbor.

Here is the output of getting "k-nearest neighbor distances" for first node for first graph in the MNIST testset.

You may notice the original code implementation (first print(..) statement result) is able to get the node with distance 0.1045919 from the first node.

--
You can verify this yourself by playing with the notebook inside visualizations dir of this repo. The .pkl file used in the notebook is from the source project here.

from benchmarking-gnns.

Thanks for your answer.

But the entries in the adj. matrix are exponentiated (gaussian kernel). Thus, the nearest neighbor corresponds to the entry with largest value, right?! In your example, it is the value 0.50751934, which isn't in the printing with the original code.

Let me reformulate my question: Why are you leaving the last column out in the piece of code below?

knns = np.argpartition(A, new_kth - 1, axis=-1)[:, new_kth:-1]

I mean... you only get the samples from new_kth up to second last value, right?!

from benchmarking-gnns.

Thanks for your answer.

But the entries in the adj. matrix are exponentiated (gaussian kernel). Thus, the nearest neighbor corresponds to the entry with largest value, right?! In your example, it is the value 0.50751934, which isn't in the printing with the original code.

Let me reformulate my question: Why are you leaving the last column out in the piece of code below?

knns = np.argpartition(A, new_kth - 1, axis=-1)[:, new_kth:-1]

I mean... you only get the samples from new_kth up to second last value, right?!

Yes, I also find this strange, I think we should change it to include the whole last part.

from benchmarking-gnns.

Off the top of my head, we put the -1 there to not include the node itself in its own k-nn neighbors set. We do the same when building TSP graphs.

@amauriholanda, can you verify/demonstrate this bug via, e.g., this notebook: https://github.com/graphdeeplearning/benchmarking-gnns/blob/master/visualization/superpixels_visualization_mnist.ipynb

from benchmarking-gnns.

Hi @chaitjo,

Thanks for the reply. I see your point. And actually that was my first guess. But you remove the node itself when you compute the adjacency matrix here (method compute_adjacency_matrix_images):

A[np.diag_indices_from(A)] = 0

Btw, this is a small thing. I don't believe it would lead to any significant difference in the results. I realized that when I independently tried to generate the MNIST data following the paper guidelines.

Anyway, I might take a look at your visualization notebook later.

Best.

from benchmarking-gnns.

We can look into this in more detail in the coming weeks and get back to you, @amauriholanda @hust-nj.

from benchmarking-gnns.

I am not sure if we investigated deeply after this discussion, but we were pretty confident that our superpixel graphs were being constructed as intended (i.e. taking the nearest neighbors instead of furthest neighbors) after we visualized them for both MNIST and CIFAR10. We have included the notebook.

Also, here is a related issue on superpixel graphs: #22

Let us know if further discussion is warranted...

from benchmarking-gnns.