Revert breaking API changes on master branch#1498
Conversation
Part of reverting breaking changes on default branch (cclib#1482)
Part of reverting breaking changes on default branch (cclib#1482)
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #1498 +/- ##
==========================================
+ Coverage 81.38% 81.41% +0.03%
==========================================
Files 73 73
Lines 14763 14789 +26
==========================================
+ Hits 12015 12041 +26
Misses 2748 2748 ☔ View full report in Codecov by Sentry. |
Part of reverting breaking changes on default branch (cclib#1482)
|
Back to trying to understand what the state of thermochemistry was before these changes. I'll check the units in the docs, but the results look like things where either implemented or fixed, not changed. Before (7ceac1a)HF/STO-3G
Contents of
|
| name | version | zpve | enthalpy | entropy | freeenergy | |
|---|---|---|---|---|---|---|
| 0 | ADF | 2007.1+200708191746 | 0.175884197049 | nan | 0.0416011560396 | nan |
| 1 | ADF | 2013.1+201309012319 | 0.176996530853 | nan | 0.0432052025783 | nan |
| 2 | DALTON | 2013.4+7abef2ada27562fe5e02849d6caeaa67c961732f | 0.177062 | nan | nan | nan |
| 3 | DALTON | 2015.0+d34efb170c481236ad60c789dea90a4c857c6bab | 0.177062 | nan | nan | nan |
| 4 | GAMESS | 8.0.1+8540 | 0.1935 | -379.575178786 | 0.0001448982107 | -379.618381321 |
| 5 | GAMESS | 2017.post1 | 0.177076 | -381.863728193 | 0.000148759506983 | -381.908081308 |
| 6 | GAMESS | 2018.post3 | 0.177076 | -381.863728193 | 0.000148759506983 | -381.908081308 |
| 7 | GAMESSUK | 7.0 | 0.1770069 | nan | nan | nan |
| 8 | GAMESSUK | 8.0+6248 | 0.1769506 | nan | nan | nan |
| 9 | GAMESS | 2018.post3 | 0.177076 | -381.863728193 | 0.000148759506983 | -381.908081308 |
| 10 | Gaussian | 2009+d.1 | 0.17714 | -382.12129 | 0.000146372631226 | -382.164931 |
| 11 | Gaussian | 2016+a.3 | 0.177132 | -382.121307 | 0.000146261948684 | -382.164915 |
| 12 | Gaussian | 2016+c.1 | 0.177132 | -382.121307 | 0.000146258594667 | -382.164914 |
| 13 | Jaguar | 7.0+207 | 0.17761484821 | nan | nan | nan |
| 14 | Jaguar | 8.3+13 | 0.177136767779 | nan | nan | nan |
| 15 | Molcas | 18.0+o180518.1800.dirty.9ead62c274ce0aa10a12879e848d75c9 | 0.1783 | -382.11385 | 0.000134031442498 | -382.153812 |
| 16 | Molpro | 2006.1 | 0.17704814 | nan | nan | nan |
| 17 | Molpro | 2012.1+c18f7d37f9f045f75d4f3096db241dde02ddca0a | 0.17746077 | nan | nan | nan |
| 18 | Molpro | 2018.2+5ea806b172b3d20b18658228ad0b1c212d33fc17 | 0.17706097 | nan | nan | nan |
| 19 | NWChem | 7.0.2+b9985dfa | 0.177031 | -382.121387735 | 0.000146436036087 | -382.165047639 |
| 20 | NWChem | 7.2.2+74936fb9 | 0.177031 | -382.121387735 | 0.000146436036087 | -382.165047639 |
| 21 | ORCA | 4.1.1 | 0.1770149 | -381.86389179 | 0.000143852154956 | -381.90678131 |
| 22 | ORCA | 4.2.0 | 0.1770149 | -381.86389179 | 0.000143852154956 | -381.90678131 |
| 23 | ORCA | 5.0.1 | 0.17701962 | -381.86823907 | 0.00014384698977 | -381.91112705 |
| 24 | Psi4 | 1!1.2.1.dev0+head.406f4de | 0.19167937 | nan | nan | nan |
| 25 | Psi4 | 1!1.3.1.dev0+head.2ce1c29 | 0.19168284 | nan | nan | nan |
| 26 | Psi4 | 1!1.7.dev0+head.6ce35a5 | 0.19168059 | nan | nan | nan |
| 27 | QChem | 5.1.2.dev0+trunk.29783 | 0.177294534322 | 0.187127055191 | 0.000146673482701 | 0.143396356323 |
| 28 | QChem | 5.4.1.dev0+trunk.37731 | 0.177294534322 | 0.187127055191 | 0.0001466718891 | 0.143396831456 |
After (110292d)
HF/STO-3G
| name | version | zpve | enthalpy | entropy | freeenergy | |
|---|---|---|---|---|---|---|
| 0 | DALTON | 2020.0+66052b3af5ea7225e31178bf9a8b031913c72190 | 0.193502 | nan | nan | nan |
| 1 | Gaussian | 2003+e.1 | 0.193501 | -379.575173 | 0.000145057856784 | -379.618422 |
| 2 | Gaussian | 2016+a.3 | 0.193501 | -379.575173 | 0.000145057856784 | -379.618422 |
| 3 | NWChem | 6.0 | 0.193409 | -379.575267894 | 0.000144981077974 | -379.618494002 |
| 4 | NWChem | 7.0.2+b9985dfa | 0.193409 | -379.575268865 | 0.000144979484373 | -379.618494498 |
| 5 | NWChem | 7.2.2+74936fb9 | 0.193409 | -379.575268865 | 0.000144979484373 | -379.618494498 |
| 6 | ORCA | 2.9.1 | 0.20747554 | -379.56441872 | 0.000118843199732 | -379.59985182 |
| 7 | ORCA | 3.0.3 | 0.19345679 | -379.57521444 | 0.000143793124266 | -379.61808636 |
| 8 | ORCA | 4.2.1 | 0.19345679 | -379.57521444 | 0.000140988495724 | -379.61725016 |
| 9 | ORCA | 5.0.4 | 0.19345678 | -379.57521445 | 0.000140988495724 | -379.61725018 |
| 10 | QChem | 5.3.1.dev0+trunk.34666 | 0.19350146094 | -379.575172811 | 0.000145057570843 | -379.618421725 |
Contents of data directory
| name | version | zpve | enthalpy | entropy | freeenergy | |
|---|---|---|---|---|---|---|
| 0 | ADF | 2007.1+200708191746 | 0.175884197049 | nan | 0.000139530961058 | nan |
| 1 | ADF | 2013.1+201309012319 | 0.176996530853 | nan | 0.000144910959511 | nan |
| 2 | DALTON | 2013.4+7abef2ada27562fe5e02849d6caeaa67c961732f | 0.177062 | nan | nan | nan |
| 3 | DALTON | 2015.0+d34efb170c481236ad60c789dea90a4c857c6bab | 0.177062 | nan | nan | nan |
| 4 | GAMESS | 8.0.1+8540 | 0.1935 | -379.575178786 | 0.0001448982107 | -379.618381321 |
| 5 | GAMESS | 2017.post1 | 0.177076 | -381.863728193 | 0.000148759506983 | -381.908081308 |
| 6 | GAMESS | 2018.post3 | 0.177076 | -381.863728193 | 0.000148759506983 | -381.908081308 |
| 7 | GAMESSUK | 7.0 | 0.1770069 | nan | nan | nan |
| 8 | GAMESSUK | 8.0+6248 | 0.1769506 | nan | nan | nan |
| 9 | GAMESS | 2018.post3 | 0.177076 | -381.863728193 | 0.000148759506983 | -381.908081308 |
| 10 | Gaussian | 2009+d.1 | 0.17714 | -382.12129 | 0.000146372631226 | -382.164931 |
| 11 | Gaussian | 2016+a.3 | 0.177132 | -382.121307 | 0.000146261948684 | -382.164915 |
| 12 | Gaussian | 2016+c.1 | 0.177132 | -382.121307 | 0.000146258594667 | -382.164914 |
| 13 | Jaguar | 7.0+207 | 0.17761484821 | -382.121011 | 0.000143519745456 | -382.163802 |
| 14 | Jaguar | 8.3+13 | 0.177136767779 | -382.121299 | 0.000146200183074 | -382.164888 |
| 15 | Molcas | 18.0+o180518.1800.dirty.9ead62c274ce0aa10a12879e848d75c9 | 0.1783 | -382.11385 | 0.000134031442498 | -382.153812 |
| 16 | Molpro | 2006.1 | 0.17704814 | nan | nan | nan |
| 17 | Molpro | 2012.1+c18f7d37f9f045f75d4f3096db241dde02ddca0a | 0.17746077 | nan | nan | nan |
| 18 | Molpro | 2018.2+5ea806b172b3d20b18658228ad0b1c212d33fc17 | 0.17706097 | nan | nan | nan |
| 19 | NWChem | 7.0.2+b9985dfa | 0.177031 | -382.121387735 | 0.000146436036087 | -382.165047639 |
| 20 | NWChem | 7.2.2+74936fb9 | 0.177031 | -382.121387735 | 0.000146436036087 | -382.165047639 |
| 21 | ORCA | 4.1.1 | 0.1770149 | -381.86389179 | 0.000143852154956 | -381.90678131 |
| 22 | ORCA | 4.2.0 | 0.1770149 | -381.86389179 | 0.000143852154956 | -381.90678131 |
| 23 | ORCA | 5.0.1 | 0.17701962 | -381.86823907 | 0.00014384698977 | -381.91112705 |
| 24 | Psi4 | 1!1.2.1.dev0+head.406f4de | 0.19167937 | -379.57027925 | 0.00013229827 | -379.60972398 |
| 25 | Psi4 | 1!1.3.1.dev0+head.2ce1c29 | 0.19168284 | -379.57027653 | 0.00013229291 | -379.60971966 |
| 26 | Psi4 | 1!1.7.dev0+head.6ce35a5 | 0.19168059 | -379.57027841 | 0.00013229523 | -379.60972224 |
| 27 | Psi4 | 1!1.7.dev0+head.6ce35a5 | 0.17709102 | -382.12398393 | 0.00014674011 | -382.16773449 |
| 28 | QChem | 5.1.2.dev0+trunk.29783 | 0.177294534322 | -382.121147522 | 0.000146673482701 | -382.164878221 |
| 29 | QChem | 5.4.1.dev0+trunk.37731 | 0.177294534322 | -382.121147521 | 0.0001466718891 | -382.164877745 |
|
I checked the docs at the old revision, and all are given as hartree/particle (or hartree/particle*kelvin for entropy). So I think all the thermochemistry changes are not breaking: the units changing for Q-Chem are actually a bugfix. |
There was a problem hiding this comment.
This is all looking great and i like this approach. I notice in the unit tests we convert the reference data while in the regression tests we convert the ccdata units. Is the reason for this just convenience? I have no issue in with it, but just wanted to make sure I understood if there was another reason..
There was a problem hiding this comment.
In both cases, the ccData object has "convenience" units, and the conversion to a.u. happens outside of the object. For the unit tests it's easy to change because you can hide this in the generic test methods that are being called for each file, and in some cases there are a lot of different reference values, so it looks even cleaner. For the regression tests, the file-specific functions don't have any generic code in them because they aren't testing common things: just because two might check scfenergies doesn't mean that attribute will be checked in a similar way, and even if it was, the number of times a generic function would be called is low compared to the unit tests.
But, it is pretty ugly. There is a clever but gross hack we can do that may still be worth doing. For the regression tests, since the ccData is instantiated via the pytest fixture mechanism, we could do the conversion to a.u. for all files right at the source:
Line 203 in 6a6c66c
regression.py. They would go back what's before this PR, with an additional comment at the top of the file that these attributes as visible on the ccData object itself, are in atomic units.
What do you think? @amandadumi @shivupa
amandadumi
left a comment
amandadumi
left a comment
There was a problem hiding this comment.
I just had a question out of curiosity that I asked, but overall these changes look good to me and thanks for finding this nice solution.
3 participants
Closes #1482
Closes #1478
Rather than revert the PRs directly, all parsing and reference data is kept in atomic units. Converting from hartree to electronvolts or wavenumbers is done after parsing. For unit and regression tests, this conversion is done in the test functions to try and keep things generic, rather than applying the conversion to the reference data.