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

simplify Vasp wrapper to minimize mangling to ASE Vasp calculator keywords #317

Merged
merged 5 commits into from
Jun 14, 2024

Conversation

bernstei
Copy link
Contributor

Initial pass removes calculator_exec and _gamma, but has to leave command_gamma.

Also slightly generalize situations under which it will use the gamma-point-only executable to include KGAMMA=True and KSPACING large enough (rather than just pbc = False).

@bernstei
Copy link
Contributor Author

@gelzinyte do you have any comments, or should I just merge?

@gelzinyte
Copy link
Contributor

Could you check and update the documentation? I think VASP is only mentioned in overview.queued.md . Looks good otherwise!

@bernstei
Copy link
Contributor Author

Could you check and update the documentation? I think VASP is only mentioned in overview.queued.md . Looks good otherwise!

Pretty sure it's still correct, but I changed the env var to ASE_VASP_COMMAND, which I actually test (locally, of course, not in the CI). Frankly, though, I'm using whole RemoteInfo dict in a an env var less these days, so maybe that example is due for a more major revision (but just as a matter of best practices, not in terms of being correct). I think I'll leave it for a different PR, though. Perhaps coupled with dropping support for my complex system for deciding which json dict to use for which function to be parallelized.

@bernstei bernstei merged commit 4c78d28 into main Jun 14, 2024
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants