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

Lattice.get_all_distance_and_image does not return ALL nearest neighbors #250

Closed
shyuep opened this issue Oct 2, 2015 · 2 comments
Closed

Comments

@shyuep
Copy link
Member

shyuep commented Oct 2, 2015

This is actually a strange bug. I am not sure if there are any major implicaitons. Maybe it does not guarantee the validity of get_all_distances.

Try the following code::

from pymatgen.util.testing import PymatgenTest
cscl = PymatgenTest.get_structure("CsCl")
gen = SlabGenerator(cscl, (1, 1, 1), 10, 10)
o = gen.oriented_unit_cell
f1 = o[0].frac_coords
f2 = o[1].frac_coords
lattice = o.lattice
print len([image for dist, image in lattice.get_all_distance_and_image(
    f1, f2) if dist < 4])
import itertools
for image in itertools.product(range(-2, 3), range(-2, 3), range(-2, 3)):
    dist, image = lattice.get_distance_and_image(f1, f2, image)
    if dist < 4:
        print image

In essence, only 6 Cl atoms were found at dist < 4 from Cs. This is because some of the nearest neighbors lie outside the range of -1 <= image <= 1.

But the get_sites_in_sphere of structure class works properly because it does not depend on this mehtod. I am just not sure if there are other methods that rely on this.

@wmdrichards Can you take a look and review?

@wmdrichards
Copy link
Contributor

I'm not entirely sure what get_all_distance_and_image is supposed to return -- as far as I can tell, its a method that you created from part of the code from get_distance_and_image. I don't think it should be its own function because the sites that it returns are entirely dependent on the lattice representation, and I'm not really sure what you'd ever want those sites for. (get distance and image is useful because it always returns the single closest image)

@wmdrichards wmdrichards assigned shyuep and unassigned shyuep Feb 1, 2016
@mkhorton
Copy link
Member

This issue seems to no longer be present, and since the example is in Python 2 also can be closed :-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants