Fix frequencyScaleFactor in Arkane examples #1657
Conversation
amarkpayne
added
Arkane
Complexity: Low
Status: Blocked by PR
Codecov Report
@@ Coverage Diff @@
## master #1657 +/- ##
=======================================
Coverage 41.71% 41.71%
=======================================
Files 176 176
Lines 29359 29359
Branches 6053 6053
=======================================
Hits 12246 12246
Misses 16244 16244
Partials 869 869Continue to review full report at Codecov.
|
amarkpayne
force-pushed
the
fix_arkane_examples
branch
from
23f41df
to
cddcc2c
Compare
| @@ -11,6 +11,7 @@ | |||
| useHinderedRotors = True | |||
| useBondCorrections = False | |||
| author = 'I.B. Modeling' | |||
| frequencyScaleFactor = 0.99 | |||
There was a problem hiding this comment.
This is a very confusing point:
The ZPE scaling factor for CBS-QB3 is indeed 0.99, but the frequency scaling factor (which Arkane requires) for CBS-QB3 is 0.99 * 1.014 =~ 1.004. I know we've all been using 0.99, we only standardized this entire scaling factors thing recently. The good news is that it's automated, so we don't need to worry about it too much. I suggest either not adding this line, or assign 0.99 * 1.014 (the latter is more transparent).
(The 1.014 is a factor between ZPE scaling and freq. scaling that is universal, according to Truhlar - almost independent of the level of theory)
There was a problem hiding this comment.
I think I'll go with your second suggestion and modify this line and the one above it to:
# Example of how to manually set the frequencyScaleFactor
frequencyScaleFactor = 0.99*1.014 # 0.99 for CBS-QB3, 1.014 for scaling between ZPE and freq. according to Truhlar
The reason I wanted to keep this line was because it is the only species example where we manually set the frequencyScaleFactor. Does this seem reasonable? If so I'll make this change and force push it
There was a problem hiding this comment.
The ref for CBS-QB3's ZPE scaling factor is dx.doi.org/10.1063/1.477924, and the ref for the 1.014 factor is dx.doi.org/10.1021/ct100326h
There was a problem hiding this comment.
Thanks! I'll add this and push the changes
amarkpayne
force-pushed
the
fix_arkane_examples
branch
from
cddcc2c
to
ae5576f
Compare
alongd
changed the title
amarkpayne
force-pushed
the
fix_arkane_examples
branch
from
ae5576f
to
2d8f2bd
Compare
|
Looks like the Travis builds are working again--I'll restart the others. @alongd is it okay if I go ahead and merge this? |
Motivation or Problem
Setting
frequencyScaleFactorin an species file instead of an Arkane input file does nothing. Therefore, we should not define this in our example files. See #1655Reviewer Note
Must be merged after #1643(Update: This PR can now be reviewed and merged when ready)