-
Notifications
You must be signed in to change notification settings - Fork 120
Addition of JDFTx code #955
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
base: main
Are you sure you want to change the base?
Conversation
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:
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). |
Hi @utf, I emailed aganose@imperial.ac.uk but didn't get a reply. Is that the correct email? Your faculty profile seems to show it as a.ganose@imperial.ac.uk. |
…is line that doesn't exist before
…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
…over for trajectory builder
… of the out file path, as well as some typing updates
… individual JEiter
… individual JEiter
…lyzing an irrelevant out file slice
…ttings of different minimization stuff
…t of redundancy here now with the jstrucs but I'm keeping it becuase I'm not 100% sure what the existing code is doing yet
Hi everyone, |
Workflow test pass
Test pass
Hi @utf |
Hi @cote3804, apologies for the delay responding on this. Overall, I think it looks great. I'd just note that in most cases, we have now moved the input sets into pymatgen directly. Would you be OK to submit a PR to transition them over? The only other thing is the jdftxcov.xml file which I think can be removed and added to the gitignore. Once those changes are made I'm happy to merge it. |
Hi @utf I'm happy to make both of those changes. Do you think it makes sense to merge the changes before moving the sets to pymatgen for the sake of having the JDFTx code working when the publication goes live? I'll still open the PR here and in pymatgen to move the sets. |
Updated dependencies in pyproject.toml
Moved JDFTx set over to Pymatgen
Moved sets to pymatgen
Hi @utf, Sorry for the long pause on this from my end. I deleted the I moved our base input set over to Pymatgen and tried to follow the VASP style as faithfully as I could. There was very little code I changed. The PR is active at materialsproject/pymatgen#4479 |
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
Future Development
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.