Hubbard: add useful utility functions
#996
Conversation
Add the possibility of finding a shell radius defining a sphere which contains the first (intersites) neighbours. This is helpful when using the `hp.x` code and having a structure containing onsite Hubbard atoms with different number of neighbours.
The Hubbard parameters can now be initialized simply by perfoming a nearest neighbour analysis (pymatgen). Standardization of the lattice and the atomic positions can also be performed to have all the atoms within the unit cell (needed for the `pw.x` routine).
Pymatgen does not order the atoms by distance. There is not apparently any such option in the API. Folding is then now sepatated from standardization (the latter could produce larger structures).
bastonero
added
type/enhancement
priority/nice to have
topic/utilities
priority/important
and removed
priority/nice to have
labels
|
For the docs probably we just need to add |
The refolding wasn't folding numbers close numerically to 1, e.g. 0.99999999. To circumvent the issue, a threshold is set, and it is possible to control it via input. Also, now the inputs for nearest neighbours are not passed via kwargs, but via an explicit dictionary input, which is then given via kwargs to the NN class.
The url for pymatgen to generate interlinks across API packages is added.
|
@mbercx @t-reents @sphuber good to have your feedbacks here. Example of usage: from aiida.orm import *
from ase.io import read
from aiida_quantumespresso.utils.hubbard import initialize_hubbard_parameters
atoms = read('./input.pwi') # anything ASE can parse
s = StructureData(ase=atoms)
hs = initialize_hubbard_parameters( # returns an initialized HubbardStructureData using nearest neighbours analysis
structure=s,
pairs={
# onsite manifold, onsite value, intersite value, dict of intersites {kind name: manifold}
'Mn': ['3d', 4.5, 1.0e-5, {'O':'2p', 'S':'2p'}],
'Fe1': ['3d', 5.0, 1.0e-5, {'O1':'2p', 'S':'2p'}],
...
},
)Cristiano and myself used it for different calculations, and the logics seem now robust. Probably not the best input API for the initialization of Hubbard parameters, but it is the price to avoid reinventing other classes which makes even more complicated to use this method, which should be easy to use and should hide the underlying complexity of the HubbardStructureData. |
| """ | ||
| rmin = 0 | ||
| pymat = self.hubbard_structure.get_pymatgen_structure() | ||
| _, neigh_indices, _, distances = pymat.get_neighbor_list(sites=[pymat[onsite_index]], r=radius_max) |
There was a problem hiding this comment.
'get_pymatgen_structure' , could be problematic if the HubbardStructureData is 1D, or 2D
There was a problem hiding this comment.
What is wrong with StructureData.get_pymatgen_structure? Is the implementation just wrong for non-periodic structures? If so, why not just fix the bug? Or is the interface fundamentally broken and to fix it, backwards-compatibility needs to be broken? If the latter, then maybe instead of breaking it, we could look into adding a correct implementation instead. On that note, @mikibonacci has been working on a new improved implementation of StructureData that addresses its current shortcomings with respect to missing properties, such as magnetic moments etc. He is developing it here: https://github.com/aiidateam/aiida-atomistic It is still under development, but it would be nice to start thinking on adding classmethods to initialize from other types (e.g. from_ase(atoms: ase.Atoms) and from_pymatgen(structure: pymatgen.core.Structure) as well as instance methods to convert the StructureDatato other types, e.g.to_ase() -> ase.Atoms` etc.
There was a problem hiding this comment.
Ok, having a look into the get_pymatgen_structure there is a strict check. This means that any pbc different from 3D would raise error. But as far as I saw in pymatgen core modules, they actually allow for different pbc (e.g. the lattice class allows any pbc).
So one should define a Lattice instance instead of passing directly the self.cell, which is simply array like. The Lattice class also provides pbc in input, so it should be straightforward.
I don't know about older version of pymatgen, maybe they didn't have this class before? Or maybe it wasn't realised at the time?
I would simply vote to do this fix in aiida-core, since this method is used also in other codes (e.g. aiida-vibroscopy, and who know what else), and it wouldn't make sense to patch this all over the places, since then also who knows what happens if settings pbc=3D in pymatgen for a 2D system.
@AndresOrtegaGuerrero we can simply open a simple PR on aiida-core and have the fix there, and you can test it. If you understood, this should be a very easy fix, so you can try it yourself as well.
There was a problem hiding this comment.
Alright, just opened the PR on aiida-core. @AndresOrtegaGuerrero it would be good if you tried to pull the new changes of aiida-core from this PR, and see if it solves also all the other issues raised in the other PR #998 of aiida-quantumespresso you opened.
| distances = distances[sort] | ||
|
|
||
| count = 0 | ||
| for i in range(len(neigh_indices)): # pylint: disable=consider-using-enumerate |
There was a problem hiding this comment.
why not using enumerate here ?
There was a problem hiding this comment.
probably it just seemed clearer to me afterwards, but definitely possible to use enumerate
| :return: (radius min +thr, radius max -thr) defining the shells containing only the first neighbours | ||
| """ | ||
| rmin = 0 | ||
| pymat = self.hubbard_structure.get_pymatgen_structure() |
There was a problem hiding this comment.
Yeah, good point, we can do that, I don't think there should be something incompatible in doing so. We just need to open an issue so we don't forget to make the code cleaner. Or actually, we can even think to override within this PR the get_pymathen_structure?
There was a problem hiding this comment.
you could in principle create the sites as PeriodicSite
sites = [
PeriodicSite(
species=site.species,
coords=site.coords,
lattice=Lattice(self.hubbard_structure.cell, pbc=self.hubbard_structure.pbc),
coords_are_cartesian=True
) for site in self.hubbard_structure.get_pymatgen().sites
]
And then instead of using the pymat StructureData , just call the function from pymatgen, that it accepts a list of periodic sites ?
get_neighbor_list(r: float, sites: Sequence[PeriodicSite] | None = None, numerical_tol: float = 1e-08, exclude_self: bool = True)→ tuple[np.ndarray, ...][source]
A set of methods are added to account for some useful use-cases where the number of nearest-neighbours are needed or when only the set of indices and the translation vector are required (the latter is useful for parsing the Hubbard parameters computed from first-principles).
Useful utility functions for
HubbardStructureDataare added:This helps issues like #991