atomistic-machine-learning/G-SchNet

A bug in get_default_neighbors func

Closed this issue · 2 comments

wxx07 commented

Hi, I found a bug in get_default_neighbors func:

def get_default_neighbors(n_atoms):
'''
Get a neighborhood indices matrix where every atom is the neighbor of every other
atom (but not of itself, e.g. [[1, 2], [0, 2], [0, 1]] for three atoms).
Args:
n_atoms (int): number of atoms
Returns:
list of list of int: the indices of the neighbors of each atom
'''
return [list(range(0, i)) + list(range(i + 1, n_atoms - 1))
for i in range(0, n_atoms - 1)]

It should be return [list(range(0, i)) + list(range(i + 1, n_atoms)) for i in range(0, n_atoms)] to return expected neighborhood indices matrix

Yes, you are right, the implementation does not fit the doc-string, thanks!

I only use the function once here:

neighbors = torch.tensor(get_default_neighbors(max_length)).long().to(device)

We only need the neighborhood list for max_length-1 atoms, but of course this should be handled when calling the function and not inside the function. So I will change these two lines such that get_default_neighbors works as you (and my doc-strings) suggested.

Fixed in b6a38d3.

Thanks again!