materialsproject / pymatgen

Python Materials Genomics (pymatgen) is a robust materials analysis code that defines classes for structures and molecules with support for many electronic structure codes. It powers the Materials Project.
https://pymatgen.org
Other
1.52k stars 864 forks source link

ZSLGenerator not returning all inequivalent transformations #2194

Open bernstei opened 3 years ago

bernstei commented 3 years ago

Describe the bug ZSLGenerator doesn't return some expected mappings between two low symmetry lattices.

To Reproduce run

from pymatgen.analysis.substrate_analyzer import ZSLGenerator

# find in-plane supercell lattice pairs that are compatible
zsl = ZSLGenerator(max_area=4.5, max_length_tol = 0.2, max_angle_tol=0.2, max_area_ratio_tol=0.2)
for match in zsl([(1, 0, 0), (0, 1.02, 0)], [(0.99, 0.0, 0.0), (0.0, 1.01, 0.0)]):
    print(match)

This only outputs one match with area ~1:

tin 1097 : python t.py  | head -20
{'film_sl_vecs': array([[1.  , 0.  , 0.  ],
       [0.  , 1.02, 0.  ]]), 'sub_sl_vecs': array([[0.99, 0.  , 0.  ],
       [0.  , 1.01, 0.  ]]), 'match_area': 1.02, 'film_vecs': array([[1.  , 0.  , 0.  ],
       [0.  , 1.02, 0.  ]]), 'sub_vecs': array([[0.99, 0.  , 0.  ],
       [0.  , 1.01, 0.  ]]), 'film_transformation': array([[1., 0.],
       [0., 1.]]), 'substrate_transformation': array([[1., 0.],
       [0., 1.]])}
{'film_sl_vecs': array([[1.  , 0.  , 0.  ],
       [0.  , 2.04, 0.  ]]), 'sub_sl_vecs': array([[0.99, 0.  , 0.  ],
       [0.  , 2.02, 0.  ]]), 'match_area': 2.04, 'film_vecs': array([[1.  , 0.  , 0.  ],
       [0.  , 1.02, 0.  ]]), 'sub_vecs': array([[0.99, 0.  , 0.  ],
       [0.  , 1.01, 0.  ]]), 'film_transformation': array([[1., 0.],
       [0., 2.]]), 'substrate_transformation': array([[1., 0.],
       [0., 2.]])}

Expected behavior Two matches that have area ~1, one that maps the first material's vector0 to the second material's vector0, and the second that maps the first material's vector0 to the second material's vector1, i.e. rotated by 90 deg. Both do have the same pair of surface cells, so maybe that's why they are not returned separately, but they are two different relative orientations, so I expected they'd each be in the list separately.

Desktop (please complete the following information): CentOS 7, anaconda3 python 3.8, pypi pymatgen 2022.0.8

bernstei commented 3 years ago

I just noticed the undocumented bidirectional argument for the ZSLGenerator constructor, but it's not clear to me how exactly it relates to this behavior. It would be nice if it could be explained in the docstring.

bernstei commented 3 years ago

I just updated to 2022.0.10, and I'm running into a new bug. I've modified my code for the change from the match object being a dict to a proper object, but now match.match_area gives an error:

Traceback (most recent call last):
  File "t.py", line 7, in <module>
    print(match.match_area)
  File "/share/apps/python/lib/python3.8/site-packages/pymatgen/analysis/interfaces/zsl.py", line 37, in match_area
    return vec_area(*self.film_sl_vectors.tolist())
AttributeError: 'list' object has no attribute 'tolist'
mkhorton commented 3 years ago

Tagging @shyamd here. A recent PR #2149 cleaned up a lot of the interface functionality and added new features, I'm not sure if this is related.

shyamd commented 3 years ago

Might be. I'll take a look tomorrow.

shyamd commented 3 years ago

The match area problem should be fixed in #2198 with an appropriate test.

bernstei commented 3 years ago

On Jul 8, 2021, at 4:29 PM, Shyam Dwaraknath @.**@.>> wrote:

The match area problem should be fixed in #2198https://github.com/materialsproject/pymatgen/pull/2198 with an appropriate test.

Thanks. Any guesses how long before the pypi version has this update, or should I just install directly from the github source if I want to take advantage of it soon?

shyamd commented 3 years ago

Probably best to just grab it off github. I don't think you're using the latest pymatgen as-is. The interfaces refactor longer returns just plain python dictionaries, but rather a dataclass object with properly named attributes to reduce ambiguities. Your output would look more like:

ZSLMatch(film_sl_vectors=[array([0.  , 1.02, 0.  ]), array([3., 0., 0.])], substrate_sl_vectors=[array([0.  , 1.01, 0.  ]), array([2.97, 0.  , 0.  ])], film_vectors=[(1, 0, 0), (0, 1.02, 0)], substrate_vectors=[(0.99, 0.0, 0.0), (0.0, 1.01, 0.0)], film_transformation=array([[3., 0.],
       [0., 1.]]), substrate_transformation=array([[3., 0.],
       [0., 1.]]))
shyamd commented 3 years ago

This should be fixed. Let me know if it's not yet.