NWChem new attributes/metadata

[amandadumi]

This is to move forward on (and will replace) #1143 .

Changes:

• This brings the changes from @jvalegre and myself, but rebased against main. (I wasn't able to figure out how to do this by pushing to the pull request, but can try again if preferred).
• implement parsing of requested metadata objects into the NWChem parser
• remove additional attributes and those that need further discussed.
• add test for vibrations in NWChem 7.

Questions/To Do:
Is it okay to introduced data attributes that are not yet parsed? (See oniom_energy and nmr_anis as examples).
Possible solutions:

• Only add the attributes for metadata that is being parsed, and separate the other unparsed attributes to a draft PR.
• Merge as is since the unparsed attributes do not introduce a breaking change and parsing can be implemented in a future PR
• Keep open until parsing of all attributes is incorporated.

Notes:
Once the newly parsed metadata attributes are merged, the cjson writer additions for metadata (#1148) can be merged.

[berquist]

I wasn't able to figure out how to do this by pushing to the pull request, but can try again if preferred

I don't think it's so bad as long as he's the author of the commit, which he still is. "Add more commits by pushing to the nwchem_fixes branch on jvalegre/cclib." means it should work. What I do is gh pr checkout 1143 and it automatically sets up the remote and its tracking branch. I have gh set to use SSH instead of HTTPS for remotes, but I think an HTTPS remote will work too if you log in via gh auth ..., not sure though. Mine looks like

$ gh auth status
github.com
  ✓ Logged in to github.com as berquist (/home/eric/.config/gh/hosts.yml)
  ✓ Git operations for github.com configured to use ssh protocol.
  ✓ Token: gho_************************************
  ✓ Token scopes: gist, read:org, repo

Is it okay to introduced data attributes that are not yet parsed?

No, because it can make the PR harder to review if they're intermingled with other new attributes that are used. That holds even if we don't have the previous problem of multiple PRs adding the same things, used or otherwise. It also means we'll be able to isolate discussions for them better and most changes are isolated to the parser(s).

My vote is for 1, "Only add the attributes for metadata that is being parsed, and separate the other unparsed attributes to a draft PR".

[berquist]

We need a test for this because these lines aren't present at all in our examples. From dvb_dispersion_bp86_d3zero.out,

      Dispersion Parameters
      ---------------------

             DFT-D3 Model
          s8 scale factor  :        1.000000000000
          sr6 scale factor :        1.683000000000
          sr8 scale factor :        1.139000000000
              vdW contrib  :        1.000000000000


             DFT-D3 Model
          s8 scale factor  :       -0.014719930985
          sr6 scale factor :

This is D3 (D3(zero), no damping), I haven't tried D3(BJ) in NWChem yet.

[amandadumi]

I see the disp vdw 3 in the input, but that is not repeated in the output from what I can tell. This might need to be changed in general. (mostly just a note to myself so i don't forget)

[berquist]

https://nwchemgit.github.io/Density-Functional-Theory-for-Molecules.html?h=disp#disp-empirical-long-range-contribution-vdw

[amandadumi]

I've added a file to test this for D3BJ with the dvb molecule, but it wouldn't be used through the typical dispersion tests on D3, but instead just to check metadata parsing. Is this overkill?

[berquist]

Sorry I missed this, I think it's ok since more people are doing D3(BJ) than D3(0).

[berquist]

What do we think about rotational temperature? #1093 (comment) definitely a method in the future, but should we parse it too?

[amandadumi]

Hm, if they are available, having them immediately available and not have to do extra work of a recalculation through a method seems like a benefit to me, but maybe I am not seeing a reason that this could be problematic?

[berquist]

It's dumb but if you apply 9f7dea1 until we remove this it'll solve the CI problem.

[berquist]

I haven't investigated but it hangs for me on parsing the vibrational frequency output.

[berquist]

int, but we also want this in bytes. I don't know if Mbytes is actually megabytes. I guess it isn't, since the number of doubles should be the number of bytes.

[berquist]

Suggested change

	self.metadata['memory'] = int(line.split()[2:])*8
	self.metadata['memory'] = int(line.split()[2])*8