-
Notifications
You must be signed in to change notification settings - Fork 103
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
A General cclib-based Taskdoc #64
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #64 +/- ##
==========================================
+ Coverage 70.39% 70.61% +0.21%
==========================================
Files 45 46 +1
Lines 3804 3972 +168
Branches 576 624 +48
==========================================
+ Hits 2678 2805 +127
- Misses 979 1005 +26
- Partials 147 162 +15
|
@utf, I still have to finish the tests, but two questions for you:
The |
Alright! We should be good to go with this, pending any suggestions you may have! With one small exception. There is a (currently commented) test I added to the VASP |
Hi @arosen93 Thanks for this. I think this is a great addition. To answer your questions:
As for the subdocuments (i.e., an I just have a couple of questions for you:
|
Thanks for the useful feedback! My questions:
Yours:
|
Before it was in a list of total energies (one for each opt step), which makes it slightly harder to query based on
Ok, before merging:
Going forward:
|
Ah, I figured out the additional_fields. Firstly, the code definitely should be # Make sure additional fields can be stored
doc = TaskDocument.from_logfile(p, ".log", additional_fields={"test": "hi"})
assert doc["test"] == "hi" |
That all sounds great to me! I'll make the suggested changes later today. I agree it's a good idea to test the jsanitization. That's gotten me more often than I'd like. And I think providing some details in the docs could definitely help, whether that be in the developer section or somewhere else. If we can put that in a separate PR, that'd be great. Would you like me to update the |
That would be perfect, thanks! |
Regarding making
If you'd like anything changed with this setup, let me know! I also added the requested jsanitization check. |
Thanks. This looks perfect. |
Hi @arosen93, I refactored this a little bit in: 0a6d616 The main changes were:
|
Thanks, that's a lot cleaner now! |
This is a PR for a general cclib-based taskdoc based on #59.
Some nice things about this include:
cclib
, especially for codes without existing infrastructure inpymatgen
TaskDocument
attributes between molecular DFT codes makes it easy to write interoperable queries.cclib
supports a large number of population analysis methods (see here). So, like thebader_caller
in Pymatgen for VASP, we now have the option to carry out many different (quick) post-processing analyses to add to theTaskDocument
following a DFT run.@utf: Let me know if you have any initial suggestions at this stage.
Here is what an example taskdoc for a Gaussian calculation on O2 looks like:
If someone wanted to do post-processing before populating the
TaskDocument
, that could be done like:If someone wanted to include the
Molecule
objects for the full optimization trajectory in theirTaskDocument
, that could be done like:This scheme should support the following codes, which are all supported by cclib: ADF, DALTON, Firefly, GAMESS, Gaussian, Jaguar, Molcas, Molpro, MOPAC, NWChem, ORCA, Psi4, Q-Chem, Turbomole.