Set kpoints in from_str method as integer in auto Gamma and Monkhorst modes
#3994
Conversation
src/pymatgen/io/vasp/inputs.py
Outdated
| @@ -1239,7 +1239,7 @@ def style(self, style) -> None: | |||
| def automatic(cls, subdivisions: int) -> Self: | |||
| """ | |||
| Constructor for a fully automatic Kpoint grid, with | |||
| gamma centered Monkhorst-Pack grids and the number of subdivisions | |||
There was a problem hiding this comment.
It should be Gamma-centered only according to the VASP Wiki?
The following KPOINTS file generates a regular Γ \Gamma -centered k-point.
This comment was marked as duplicate.
This comment was marked as duplicate.
…r parts (#3996) * docstring tweaks * fix typo * reduce indentation level * fix vasp case to VASP * use walrus operator * revert overlapping functional changes from #3994 * my bad, I get confused hopping between two PRs * remove debug code from vasp.help * fix `use-named-expression` with sourcery * clean up Vasprun.as_dict * simplify dict generation * re-raise and update -> |= * simplify logic conditions * remove unused logger * remove a lot of unused logger, wondering if they exist for a reason? * remove some unused module_dir, they must have gone stranded * CAPS LOCK ENGAGED: Go up! module level variables!
|
@janosh Could you please comment and review this PR? Currently the type of Kpoint is a bit confusing (mainly around the existing of |
| kpoints = Kpoints.monkhorst_automatic((2, 2, 2), [0, 0, 0]) | ||
| assert kpoints.style == Kpoints.supported_modes.Monkhorst | ||
| assert kpoints.kpts == [(2, 2, 2)] | ||
| kpoints = Kpoints.automatic(100) | ||
| assert all(isinstance(kpt, int) for kpt in kpoints.kpts[0]) |
There was a problem hiding this comment.
could be refactored to {*map(type, kpoints.kpts[0])} == {int} but maybe less readable
There was a problem hiding this comment.
Yes it looks less readable, also I guess type checking with type(var) == type_of_var feels a bit off?
I don't think kpt would be a subclass of int any time soon so it would be safe, but still not sure if we want to change this. I would let you decide :) Thanks for the comment.
Meanwhile {*map(lambda kpt: isinstance(kpt, int), kpts)} == {True} would work, but...well...It's way over-complicated ;)
| if len(_kpt) != 3: | ||
| raise ValueError("Invalid Kpoint length.") | ||
| kpt: Tuple3Floats = cast(Tuple3Floats, tuple(_kpt)) | ||
| kpt: Tuple3Ints = cast(Tuple3Ints, tuple(_kpt)) |
There was a problem hiding this comment.
i think the : Tuple3Ints is redundant if you cast anyway?
There was a problem hiding this comment.
Yes it's indeed redundant but I kept it for readability for humans (in case someone don't know about cast). Feel free to remove it if you like :)
janosh
changed the title
kpoints in from_str method as integer in auto Gamma and Monkhorst modes
|
Thanks @janosh |
Summary
Kpoints.from_strmethod as integer in auto Gamma-centered and Monkhorst-Pack modes, to fix Kpoints Sampling Saved as Float When Writing Kpoints file #3993It turns out typing
Kpointas a sequence offloatis giving us quite some headache and confusion:pymatgen/src/pymatgen/util/typing.py
Line 63 in b28c937
It was originally done this way for compatibility with line-mode and explicit kpoint mode (when typing,
floatacts asUnion[int, float]instead of justfloat). Is there any other cases whereKpointcould befloatthat I forgot to mention?Inputs and comments are hugely appreciated.