-
Notifications
You must be signed in to change notification settings - Fork 875
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
FHI-aims IO Parsers #3435
FHI-aims IO Parsers #3435
Conversation
1) parsers and AimsOuput ojbects moved over 2) All ASE Atoms objects removed in favor of pymatgen structures 3) TO DO: Check name schemes 4) Create an AimsInputs object in the future
1) remove aims_outputs naming convention 2) Add AimsGeometryIn and AimsControlIn classes
Adjusted from ASE parser tests
1) How to handle stress in properties?
For aims outputs checks
Test for both Si and H2O
Make sure that the AimsCubes are properly formatted
Add test for aims control.in file generators
as requested all output files are gzipped
1) Species blocks can now read from gzipped file 2) geometry.in can now read a gzip file
Error in the Actions from rounding errors, do more explicit test for as_dict
1) Docstrings are googledoc format 2) All type hinting is done
Everything is also type hintted now
I think everything is now typehinted with google docstrings |
@janosh I think this is ready to review as well |
Tests should now work |
I updated to master if those changes are needed for the docker images (errors are in tests I did not touch) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice work @tpurcell90! 👍 🚀
Left a few comments.
1) Moved lines objects to seperate files 2) inline k_point_weights 3) Remove trailing comma 4) tmpdir -> tmp_path
Do we need to write a tutorial for the aims IO or are the docstrings good enough? |
Also do I update the ChangeLog in a commit or is this done by the maintainers? |
You are very welcome to write a tutorial! 👍 Were you thinking a Jupyter notebook? I've been thinking about starting a
We'll handle that. |
use numpy eye instead of a full list Co-authored-by: Janosh Riebesell <[email protected]> Signed-off-by: Thomas Purcell <[email protected]>
Ya I was thinking a basic jupyter notebook. I'll take a look at the examples tomorrow and make one up in the office (I am doing data transfer so I am not bringing my work computer home with me this week) |
Keep it consistent with inputs naming scheme
I added an example file. I think everything is finished now |
1) Remove copy of species file in examples 2) Typo corrections in docstrings 3) d -> dct for `from_dict` methods 4) error mesages for AimsCube 5) Shortened default checks in Inputs 6) Removal of chdir in tests
…ut_files tests/io/aims/(aims_->'')parser_checks
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks so much @tpurcell90! This was a big PR with nice docs and excellent test coverage, esp. for a 1st-time contributor! 👍
No problem happy to add this into pymatgen! @janosh thanks for reviewing this! |
aims.output -> aims.outputs module name
Head branch was pushed to by a user without write access
Corrected the module name in the test ref files, all tests should now pass. I forgot to do a final clean install of pymatgen when I made the names of the modules consistent. |
Summary
Creates an FHI-aims interface for pymatgen based off of the ASE parser and input generating structures. Part of this work was done in atomate2 originally for this PR (materialsproject/atomate2#562) where @ansobolev contributed.
This PR was made by the request of atomate2 so the IO can remain with pymatgen and not in that repoistory.
Major changes:
Todos
Checklist
ruff
.mypy
.duecredit
@due.dcite
decorators to reference relevant papers by DOI (example)Tip: Install
pre-commit
hooks to auto-check types and linting before every commit: