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

Addition of JDFTx code #955

Open
wants to merge 264 commits into
base: main
Choose a base branch
from
Open

Addition of JDFTx code #955

wants to merge 264 commits into from

Conversation

cote3804
Copy link

@cote3804 cote3804 commented Aug 13, 2024

Summary

Adding support for JDFTx, an open source plane wave code that supports grand-canonical DFT and implements accurate solvation models.

Dependencies on other repositories

What we've done

  • Written JDFTx input and output parsers in pymatgen/io
  • Implemented base input set and generator plus three base input sets
  • Implemented basic calculation input and output schema
  • Written tests following CP2K and VASP format

Future Development

  • Make JDFTx compliant with the adsorbate workflows
  • Add more rich schemas and store custodian metadata in schemas

Developer Note

We are working with @mkhorton on this project. We are the developers behind the BEAST Database and have extensive experience using JDFTx for high-throughput workflows as well as direct collaboration with the developer of JDFTx.

@utf
Copy link
Member

utf commented Aug 14, 2024

Hi @cote3804, this is fantastic. Thank you for contributing it.

Firstly, would you be able to send me an email? My address is aganose [at] imperial [dot] ac [dot] uk.

For the PR, here are some high level comments to start with:

  • The best place for the IO functionality is pymatgen. Are you happy to submit a PR there once the input sets are completed? I'm happy for you to keep them in this PR while you work on finalising the workflows, as I realise this makes it easier for testing etc.
  • You can rename the emmet folders to schemas inline with atomate2 convention.
  • The READMEs and test files inside the atomate2/src directory aren't ideal. We can add a JDFTx page to the documentation (in docs) and the test files should go in tests (although currently it looks like most of these files will be in pymatgen anyway).
  • To help conform with the atomate2 code style, can you install pre-commit and run it on all of your files. This gets called automatically as part of the GitHub CI and the tests won't pass until all style issues are resolved.
pip install pre-commit
pre-commit run -all

Once you're ready for me to look at the input sets and jobs/flows please let me know (currently they have a lot of VASP code still).

@cote3804
Copy link
Author

Hi @utf,

I emailed [email protected] but didn't get a reply. Is that the correct email? Your faculty profile seems to show it as [email protected].

benrich37 and others added 27 commits September 3, 2024 12:58
…ady. It takes a .in file and runs the jdftx calculation using a command (Docker command is used atm) and outputs an out file and any requested output. Example and testing are in sample-move-later folder.
Initial Jobs scripts via custodian
Addede schemas and new JDFTXOutfile methods
…G15" appears in the file path of the pseudo file. If both appear, it chooses whichever appears later in the path. If neither, raises value error
…self._set_pseudo_vars, which will eventually be able to fill the same fields for GBRV pseudopotentials
cote3804 and others added 29 commits December 3, 2024 05:44
Changing core sets and TaskDoc
Fixed charges, forces, and stress atributes in the calculation docs
Merging latest changes in upstream.
@soge8904
Copy link

Hi everyone,
I know this PR was previously blocked because it depended on materialsproject/pymatgen#4189 and materialsproject/pymatgen#4190. That dependency has now been merged, so I wanted to check if this PR can be reviewed again.
I’ve updated the branch with the latest changes from main, made sure pre-commit checks pass, and verified that the tests run correctly. Let me know if anything else needs to be addressed!

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.

5 participants