Gaussian new attributes and metadata - #1269

@oliver-s-lee let me know what files are required, I believe most of the new attributes can be tested with files that already exist in the data folder

@jvalegre thanks for making the split! I'll review and make detailed comments, but it looks like there's some great stuff in here so thanks in advance.

I know this has been waiting for a while, so I'll make suggestions directly to the code where relevant so we can move along more quickly.

A few general comments:

• cclib tends to use lowercase letters only for attribute names, we tend to avoid camel case or underscores (eg nmriso rather than nmr_iso)
• metadata is normally stored in the metadata dictionary rather than as its own attribute. I'f you're mostly using the cjson extractor then this technicality won't be obvious, but I'll update in the code to show how it works

Metadata support for cjson is being worked on in #1148, so I'll propose removing duplicated metadata stuff from this PR

@oliver-s-lee I left some comments open for you to review, let me know your thoughts and have a nice weekend!

Thanks for this contribution @jvalegre and thanks @oliver-s-lee for handling the review so far. I wanted to weigh in regarding :

I'm not sure, metadata doesn't get added to the json file when using ccwrite json FILENAME
and
P.S. this is important, since we use ccwrite in AQME to get properties

Let's definitely proceed with having cclib write these to the "correct" location in a cjson file and modifying the standard upstream if needed as was done by @amandadumi here. A logical serialization is preferred to any work-arounds. This PR and the project demonstrate the need any modifications to cjson, so changes are justified upstream.

I will take a closer look this weekend. I have been thinking about S^2 parsing so I'd like to take a closer look at how things are being stored here. I'd like the container to be general enough to handle S^2 from multiple levels of theory/ multiple states.

I'm more than happy to improve the spec and/or the avogadro-cclib plugin

One thing I note, is that the metadata is really inconsistent across parsers.

• Gaussian gives functional but Orca and GAMESS do not
• Orca gives me a list of all keywords
• NWChem gives me 'functional': 'Becke', for dvb_dispersion_bp86_d3zero
• GAMESS gives me 'methods': ['RHF'] for dvb_dispersion_bp86_d3zero
• etc.

I'd really like to be able to read an output file and then re-use those methods / keywords in the input generators.

Hi @jvalegre thanks for the replies, I'll go through them when I get a chance.

On a general note, I'm not sure how the workflows in AQME work, but if it's written in python I wonder if it might be worth accessing the cclib data directly, rather than going through the cjson writer?

For example, you can do:

import cclib.io
data = cclib.io.ccread("file.log")
print(data.metadata['symmetry_used'])
c1

That way you don't need to wait for the cjson updates, and can access the data directly.

@oliver-s-lee completely right, I could adapt the code that way and create json files afterwards

Also worst case scenario if we modify the writing of the cjson file here to write the attributes to the correct places even if those are currently non-compliant with the standard, then the cjson will still be created.

@ghutchis good point I'll open an issue about this and we can try to unify the things in metadata.

The serialization issues and the new attribute issues are difficult to separate in this PR. To move this forward and the other open PRs (#1141, #1142, #1143), we need agreement first on the new attributes followed by upstream PRs for where attributes should be saved. This process can take longer since it will require upstream PRs, but if we can get agreement on the changes required for cclib then the downstream project can have custom ccData->json dumps following this. This would essentially require creating a dictionary mapping json locations to ccData attributes. This would allow the PRs to move faster since we can't output non-complaint cjson.

hi @oliver-s-lee - just catching up, everything in order with the PR? Let me know if I missed any comments

Hi @jvalegre, we're mostly there I think in terms of the parsing I think! The main thing we're missing now is tests, which I will work on adding when I get time. If you've got an experience on writing automated tests and would be able to help that would certainly expedite things.