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 868 forks source link

pmg.io.vasp.Kpoints.from_string() problem at picking up the IBZKPT style input #224

Closed terencezl closed 9 years ago

terencezl commented 9 years ago

This is what pmg.io.vasp.Kpoints has in 3.1.5.

        #Assume explicit KPOINTS if all else fails.
        style = Kpoints.supported_modes.Cartesian if style == "ck" \
            else Kpoints.supported_modes.Reciprocal
        kpts = []
        kpts_weights = []
        labels = []
        tet_number = 0
        tet_weight = 0
        tet_connections = None

        for i in range(3, 3 + num_kpts):
            toks = lines[i].split()
            kpts.append([[float(j) for j in toks[0:3]]])

Should the if style == "ck" be changed to in "ck"? And kpts.append([[float(j) for j in toks[0:3]]]) changed to kpts.append([float(j) for j in toks[0:3]])? The list level is one more than being correct.

Also, pmg.symmetry.bandstructure.HighSymmKpath().get_kpoints() returns the kpoints as a list of bunch of numpy arrays. It feels unnecessary and inconsistent for the use of numpy array here on explicit kpoints, after it's been calculated and generated, of course.

P.S. I was intending to manually construct a new kpoints file from the DFT IBZKPT and the line kpoints for the hybrid run.

shyuep commented 9 years ago

Thanks. I have fixed it the explicit kpoint parsing. The get_kpoints doesn't really matter since numpy arrays are effectively lists for the most part. Is there a specific use case you find a problem with the numpy arrays?

terencezl commented 9 years ago

No, not at all. It (kpoints.kpts) just didn't print out uniformly if there are lists and arrays. That's all. But the generated kpoints file is good.

So I kept trying to work the hybrid band gap calculation out, and noticed a few things.

  1. In pmg.io.vasp.Vasprun.get_band_structure, there seems to be some work under progress, with a TODO list "make a decision on the convention with 2*pi or not." I assume it's related with VASP's cartesian k-points 2 pi/a notation. Structure.to('POSCAR') writes a file with a = 1, so the only factor is the 2 pi. But that doesn't make the reciprocal lattice 2 pi times more. Of course, if used for plotting, it doesn't really matter, because the scale is relative. But is there a reason behind this?

       lattice_new = Lattice(self.lattice_rec.matrix * 2 * math.pi)

    And it doesn't consider cartesian input.

           return BandStructureSymmLine(kpoints, eigenvals, lattice_new,
                                        efermi, labels_dict,
                                        structure=self.final_structure,
                                        projections=p_eigenvals)
       else:
           return BandStructure(kpoints, eigenvals, lattice_new, efermi,
                                structure=self.final_structure,
                                projections=p_eigenvals)

    While the instantiation syntax of BandStructure/BandStructureSymmLine is

    def __init__(self, kpoints, eigenvals, lattice, efermi, labels_dict,
                coords_are_cartesian=False, structure=None,

    If cartesian k-points are considered at all, since this method belongs to the vasp io, the actual kpts need to be multiplied by 2 pi.

  2. The reason I was caught up with it, is that pmg.symmetry.bandstructure.HighSymmKpath.get_kpoints currently returns only cartesian coordinates. (and it's not in the format of VASP, but that's good, because pmg.symmetry.bandstructure.HighSymmKpath should be QM code agnostic.)
  3. pmg.electronic_structure.plotter.BSPlotter.get_plot has line rc('text', usetex=True), which implicitly depends on a Latex installation, and changes the matplotlib environment, but the current label setup doesn't quite need the external Latex support. Can it fall back to matplotlib's Tex if there is no Latex installed?
shyuep commented 9 years ago

For this, I will direct your question to @hautierg, who was the one who implemented the Bandstructure algorithms and plotting. For (3), I would say yes, we should make sure that Latex installation is not a pre-req. Geoffroy, can you make the changes?

hautierg commented 9 years ago

Hi Terence,

Thanks for your comments and for pointing this out. There are two conventions for reciprocal space vectors: the physics one with a 2pi factor and the crystallography one (without). Pymatgen uses the physics one. This 2pi factor was not necessary (as already taken care by pymatgen.lattice), I have removed it. As for running HSE band structures with pymatgen, no need to read the IBZKPT. You could generate the k-point file directly using MPBSHSEVaspInputSet(). I have just fixed a problem in there that was related to your question of HighSymmKPath returning cartesian kpoints. Let me know if you have any other questions. I will look at the tex problem now.

terencezl commented 9 years ago

I see. Wish there is a tutorial using the input sets. It looks like a junction between pymatgen and fireworks. Pymatgen provides the inputs, while fireworks defines the workflow.

I don't want to intrude, but sometimes I just needed a small implementation, and use a few core objects and functions from pymatgen, rather than the sets, at least before they are more accessible. That's why I wish I could call pmg.symmetry.bandstructure.HighSymmKpath.get_kpoints(kpoints_division, return_cartesian=False) and get the fractional kpts.

Fractional kpts is more flexible, isn't it?

hautierg commented 9 years ago

I agree that the user should be able to use cartesian or fractional at the HighSymmKpath level. I have modified the code to do so.

computron commented 8 years ago

There were a lot of items mixed into this issue and I think the LaTeX part got left behind. On my current system I have to set rc('text', usetex=True) to be rc('text', usetex=False) to get it to work (the plot renders fine). Not sure why I get the error in the first place since I have LaTeX installed. I can probably get it working on my system by fiddling around but it would be better for pymatgen overall if the original suggestion to not require tex installation.

shyuep commented 8 years ago

I pushed a fix to this by adding a try catch on the rc. In general, I would strongly recommend against using rc to set anything. That really should be left to the user to set it since it is a global matplotlib setting.