materialsproject
Make CrystalNN work with fractional occupation
When the CrystalNN neighbor-finding method is applied to a structure with fractional occupation (in the test case from a CIF file), it exits with an attribute error:
File "pymatgen/pymatgen/analysis/local_env.py", line 4010, in get_cn return super().get_cn(structure, n, use_weights) File "pymatgen/pymatgen/analysis/local_env.py", line 265, in get_cn siw = self.get_nn_info(structure, n) File "pymatgen/pymatgen/analysis/local_env.py", line 3842, in get_nn_info nndata = self.get_nn_data(structure, n) File "pymatgen/pymatgen/analysis/local_env.py", line 3905, in get_nn_data X1 = structure[n].specie.X File "pymatgen/pymatgen/core/sites.py", line 80, in getattr raise AttributeError(a) AttributeError: specie
Removing the fractional occupation in the underlying CIF file results in no problems.
Maybe the fractional occupation should simply be ignored for determining the neighbors and coordination numbers. The only issue that remains is then when fractional site a close to each other so that they might be deemed coordinated when the question might be: is either site 1 or site 2 present, not both so that they cannot be coordinated.
No. That is the wrong approach. The basic premise is that people should code for disordered structures, rather than assume that all structures will always be ordered. In this case, specie was the wrong attribute to use in the first place. Feel free to just change the code to work with disordered structures and submit a PR.
@mkhorton Let's make sure when people submit PRs in the future that we reject anything that assumes a structure is ordered without good reason. The only cases where full occupancies should be enforced are when interfacing with computational codes that require ordered structures. All generic analysis must work with ordered and disordered structures.
There's a logical difficulty here, in that edges/bonds are defined with respect to sites, and basically all bonding algorithms require there to be a specific atom at each end of the bond. In disordered structures, there's going to be a fundamental ambiguity present.
A general "fix" for this might be in the NearNeighbors class (i.e. not CrystalNN specifically) to calculate bonding only with the majority member of that site. This is still a compromise.
@shyuep sure, I think enforcing tests with disordered structures where appropriate would be useful -- we shouldn't be seeing an AttributeError here, if the logic doesn't work for disordered structures we should simply be told that.
Stumbled into the above error too just now.
to calculate bonding only with the majority member of that site. This is still a compromise.
What about using a weighted average?
What about using a weighted average?
I agree this should be an acceptable workaround.
