pymatgen icon indicating copy to clipboard operation
pymatgen copied to clipboard

ZSLGenerator not returning all inequivalent transformations

Open bernstei opened this issue 3 years ago • 8 comments

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 avatar Jul 08 '21 03:07 bernstei

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 avatar Jul 08 '21 13:07 bernstei

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'

bernstei avatar Jul 08 '21 13:07 bernstei

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.

mkhorton avatar Jul 08 '21 17:07 mkhorton

Might be. I'll take a look tomorrow.

shyamd avatar Jul 08 '21 17:07 shyamd

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

shyamd avatar Jul 08 '21 20:07 shyamd

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?

bernstei avatar Jul 08 '21 20:07 bernstei

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 avatar Jul 08 '21 20:07 shyamd

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

shyamd avatar Sep 13 '21 08:09 shyamd