added attribute nfrozenatom and altered disps loop to only loop over … #1286
Conversation
|
Do you have an example file? I'm pretty sure this would also break a lot of other parsers. I'm thinking of Open Babel at the moment, but adding a test file would be important. |
|
Hi Geoff,
Yeah, of course.
I have uploaded an example file to my branch of the forked repo: https://github.com/hklem/cclib/tree/gaussianparser_atomflags
It should be in data/Gaussian/basicGaussian16/frozen_atom_flags.log
Let me know if you have any issues.
…--
Heidi
From: Geoff Hutchison ***@***.***>
Date: Tuesday, October 10, 2023 at 6:17 PM
To: cclib/cclib ***@***.***>
Cc: Klem, Heidi R. (Fed) ***@***.***>, Author ***@***.***>
Subject: Re: [cclib/cclib] added attribute nfrozenatom and altered disps loop to only loop over … (PR #1286)
Do you have an example file? I'm pretty sure this would also break a lot of other parsers. I'm thinking of Open Babel at the moment, but adding a test file would be important.
—
Reply to this email directly, view it on GitHub<#1286 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/AKXVVXLE7ZZVX5GBPEHJBTLX6XQQDAVCNFSM6AAAAAA5255VLSVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTONJWGQ4TGNRQGE>.
You are receiving this because you authored the thread.Message ID: ***@***.***>
|
There was a problem hiding this comment.
Thanks for this fix. My concerns are, since I'm not familiar with the frozen atom feature,
- Keeping track of which atoms are frozen. We should store their indices (or at least have the Gaussian input file kept) in
metadata. This way we don't have to worry about if the indices are contiguous, etc. - I agree about testing other things that consume
ccdataobjects, like the Open Babel bridge, in part because it isn't clear which atoms are frozen, but also just general consistency. A test for this can be added to https://github.com/cclib/cclib/blob/master/test/regression_io.py (making a newregression_bridge.pymay be better, not sure). - The actual location of this output should go in https://github.com/cclib/cclib-data/ and a test for
vibdispslength in https://github.com/cclib/cclib/blob/master/test/regression.py.
cclib/parser/gaussianparser.py
Outdated
| while line.split() and not "Variables" in line and not "Leave Link" in line: | ||
| natom += 1 | ||
| if line.split()[1] == '-1': |
There was a problem hiding this comment.
This is causing failures in other files (water_ccsd.log) where there aren't at least two tokens in a line.
There was a problem hiding this comment.
I'm not very familiar with Open Babel. Maybe you can give me more information about how that program interfaces with ccdata and how we should go about making sure the nfrozenatom and frozenatoms attributes don't cause issues.
|
@hklem poke. |
|
Thanks for the poke. I have revised to fix the failures and set two new attributes, 'nfrozenatom' and 'frozenatoms' in gaussianparser.py. self.frozenatoms is a list of atom indices with the '-1' frozen flag (this is 1 indexed to be consistent with Gaussian since it begins numbering atoms at 1). The definitions have been added to data.py. I have changed the way frozen atoms are accounted for in a way that I believe is more intuitive, but this is based on my assumption of what the 'nqmf' attribute is. I'm not familiar with what this term means in a Gaussian output. I see it gets defined in water_ccsd.log. My best guess is number of atoms with QM forces? I looked at all the Gaussian16 files from cclib/data and cclib-data/ and in the files where NQMF is in the output it is always =0. I'm not sure what the situation would be if nqmf is defined and is >0, but the way I edited the code if that does occur it should have exactly the same behavior as without my changes, and the frozen atom attributes won't even be set. I added code to regression.py perform two tests: one on the frozen_atom_flags.log (needs to be added to cclib-data) to ensure they are properly parsed and one on water.log to ensure the frozen atom attributes aren't stored and that the vibdisps are parsed properly. |
…atoms that have frequency information.
e9d7117 to
6008492
Compare
|
This PR is almost a year old. I will revisit it soon to try and figure out what the path forward should be. |
…atoms that have frequency information.
When using the '-1' atom flags in the molecule specification to freeze atoms during gaussian calculation the gaussian output file does not list those atoms in the vibrational frequency section. This causes issue with gaussianparser.py which loops over self.nqmf (or natoms) to store 'numbers' into self.vibdisps. The error will appear because natoms > the number of lines with that data, so the loop will run over into the next lines. I changed this behavior by setting a self.nfrozenatom attribute that is defined at the same time as self.natoms. Now the loop in question goes over the range self.nqmf-self.nfrozen atoms.