Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Kpoints objects using wrong mesh types? #3220

Closed
matthewkuner opened this issue Aug 4, 2023 · 9 comments
Closed

Kpoints objects using wrong mesh types? #3220

matthewkuner opened this issue Aug 4, 2023 · 9 comments
Labels
io Input/output functionality needs discussion Needs discussion to agree on actionable next steps vasp Vienna Ab initio Simulation Package

Comments

@matthewkuner
Copy link
Contributor

matthewkuner commented Aug 4, 2023

if has_odd or is_hexagonal or force_gamma:

Currently, this code could use Monkhorst-pack schemes for FCC and Face-centered orthorhombic cells. According to the vasp wiki, this is not good...

Anyone like @janosh @munrojm @esoteric-ephemera know why this is?

@janosh janosh added io Input/output functionality needs discussion Needs discussion to agree on actionable next steps vasp Vienna Ab initio Simulation Package labels Aug 4, 2023
@janosh
Copy link
Member

janosh commented Aug 4, 2023

Tagging @arosen93 as the author.

@matthewkuner matthewkuner changed the title Kpoints objects using wrong mesh types Kpoints objects using wrong mesh types? Aug 4, 2023
@esoteric-ephemera
Copy link
Contributor

Just to copy over part of my discussion with Matthew here: if a Monkhorst-Pack grid doesn't break the symmetry of the lattice, then everything should be OK. I did check the "gold-standard" MP entry, mp-149 / cubic diamond Si, and the input for the task document has a Monkhorst-pack grid (inappropriately)

VASP will throw an error if the breaking of the lattice point / space group symmetry is severe enough. Custodian should catch this

The easy resolution is if the space group of a structure starts with 'F', force a gamma-centered k-point mesh

@Andrew-S-Rosen
Copy link
Member

What am I the author of? 😅 I don't recall touching this.

@shyuep
Copy link
Member

shyuep commented Aug 4, 2023

@arosen93 Your reputation is such that you are the author of everything ! :-)

@matthewkuner
Copy link
Contributor Author

matthewkuner commented Aug 4, 2023

@arosen93 It looks like you did actually write this almost 2 years ago lol: 8648d50

However, it looks like similar code existed prior to this.

@Andrew-S-Rosen
Copy link
Member

Andrew-S-Rosen commented Aug 4, 2023

True, I admittedly did add the automatic_density_by_length function, which is basically a minor variation of automatic_density_by_vol for systems with vacuum in c. I didn't want to touch any of the existing mesh logic. Since there's some repeated code, it probably would have been better to re-factor it a bit to prevent duplication, but alas I was young(er) and naïve(r). Thanks for cleaning this up!

Personally, I don't know much about the Monkhorst-Pack vs. Gamma-centered criteria in VASP (and thought at some point we decided to do Gamma-centered for all MP calculations, but I suppose not). I'll defer to @esoteric-ephemera on the best course of action on this one.

Edit: I should note that automatic_density_by_length isn't used by any MP-related workflows, although the pre-existing automatic_density_by_vol is.

@matthewkuner
Copy link
Contributor Author

matthewkuner commented Aug 4, 2023

Yeah it seems like this code existed in some form at least since 2015: 423aa16#diff-2b61e131c3f0a2a133effdfb20a2b8b6c84690d55bcd81b6d01ea4ecebdb9b0dL902

so basically since this file was created... this could potentially be a very large issue for calcs pre-dating force_gamma = True being added to Atomate2 calcs.

There are definitely a sizeable number of calcs in the MPDB using Monkhorst meshes.

@janosh
Copy link
Member

janosh commented Aug 4, 2023

@arosen93 My bad, I thought we were still talking about automatic_density_by_lengths added in 8648d50. 😅

@Andrew-S-Rosen
Copy link
Member

Happy to take the blame regardless. 😉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
io Input/output functionality needs discussion Needs discussion to agree on actionable next steps vasp Vienna Ab initio Simulation Package
Projects
None yet
Development

No branches or pull requests

5 participants