Parse CCSD(T) energies (no F12) in Molpro #1592

alongd · 2019-05-02T18:17:57Z

I found out that some jobs don't have the Electronic Energy at 0 [K]: -768.275662 [H] line we're currently looking for when parsing energies from a non-F12 CCSD calculation in Molpro. I made a test out of this example, and modified the parser accordingly.

codecov · 2019-05-02T19:01:07Z

Codecov Report

Merging #1592 into master will decrease coverage by 0.01%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master    #1592      +/-   ##
==========================================
- Coverage    41.6%   41.58%   -0.02%     
==========================================
  Files         176      176              
  Lines       29147    29147              
  Branches     5995     5995              
==========================================
- Hits        12127    12122       -5     
- Misses      16183    16187       +4     
- Partials      837      838       +1

Impacted Files	Coverage Δ
rmgpy/data/kinetics/family.py	`52.74% <0%> (-0.24%)`	⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8c2f86a...20ff313. Read the comment docs.

cgrambow

Making the PEP-8 changes as well is fine, but could you also make a separate commit for PEP-8 changes in molpro.py so that your actual code change in your first commit is clear?

cgrambow · 2019-05-02T19:33:17Z

arkane/molpro.py

+            # Determine whether the sp method is f12,
+            # if so whether we should parse f12a or f12b according to the basis set.
+            # Otherwise, check whether the sp method is MRCI.
+            f12, f12a, f12b, mrci = False, False, False, False


Why do you need the f12 variable? Could you just check that both f12a and f12b are false?

We determine f12a and f12b from the basis set, so one of them could be True, but the method won't necessarily be F12

I see, but we could also just set f12a to be true if we encounter both f12 and vtz/vdz in the file and likewise for f12b.

They don't have to be on the same line if one uses an F12 method but not an F12 basis set (for which we have model chemistries)

cgrambow · 2019-05-02T19:34:02Z

arkane/molpro.py

                        break
-            if E0 is None and mrci:
+                elif not f12:
+                    if 'Electronic Energy at 0' in line:


Just curious, do you know when (if ever) this gets used?

Not really. This is actually the original line we were parsing so far for non-F12 sp jobs. The only thing "different" about the test I added is that it is QZ (basis=aug-cc-pvqz). Maybe that changes something in the output format.

cgrambow · 2019-05-02T19:35:34Z

arkane/molproTest.py

-        log=MolproLog(os.path.join(os.path.dirname(__file__),'data','ethylene_f12_qz.out'))
-        E0=log.loadEnergy()
+        log = MolproLog(os.path.join(os.path.dirname(__file__), 'data', 'ethylene_f12_qz.out'))
+        eo = log.loadEnergy()


cgrambow · 2019-05-02T19:35:41Z

arkane/molproTest.py

+        eo = log.loadEnergy()

-        self.assertAlmostEqual(E0 / constants.Na / constants.E_h, -78.472682755635, 5)
+        self.assertAlmostEqual(eo / constants.Na / constants.E_h, -78.472682755635, 5)


alongd

Thanks! force pushing fixes

alongd · 2019-05-02T20:08:10Z

arkane/molpro.py

+            # Determine whether the sp method is f12,
+            # if so whether we should parse f12a or f12b according to the basis set.
+            # Otherwise, check whether the sp method is MRCI.
+            f12, f12a, f12b, mrci = False, False, False, False


We determine f12a and f12b from the basis set, so one of them could be True, but the method won't necessarily be F12

alongd · 2019-05-02T20:09:56Z

arkane/molpro.py

                        break
-            if E0 is None and mrci:
+                elif not f12:
+                    if 'Electronic Energy at 0' in line:


Not really. This is actually the original line we were parsing so far for non-F12 sp jobs. The only thing "different" about the test I added is that it is QZ (basis=aug-cc-pvqz). Maybe that changes something in the output format.

cgrambow · 2019-05-03T17:22:32Z

arkane/molpro.py

+                        e0 = float(line.split()[-1])
                        break
                    if 'Electronic Energy at 0' in line:
                        E0 = float(line.split()[-2])


You forgot to change some of the E0s.

cgrambow · 2019-05-03T17:24:17Z

arkane/molpro.py

+
    def __init__(self, path):
-        self.path = path
+        super(MolproLog, self).__init__(path)


This isn't a PEP-8-related change. Add to commit where you do the same for Gaussian and Q-Chem.

cgrambow · 2019-05-03T17:26:15Z

arkane/molpro.py

        negativefrequency = -float(frequency)
        return negativefrequency
+
+    def loadScanEnergies(self):


Also not a PEP-8-related change.

cgrambow · 2019-05-03T17:30:48Z

arkane/molpro.py

+            # Determine whether the sp method is f12,
+            # if so whether we should parse f12a or f12b according to the basis set.
+            # Otherwise, check whether the sp method is MRCI.
+            f12, f12a, f12b, mrci = False, False, False, False


I see, but we could also just set f12a to be true if we encounter both f12 and vtz/vdz in the file and likewise for f12b.

cgrambow · 2019-05-03T17:35:36Z

arkane/molpro.py

                        f12a = True  # MRCI could also have a vdz/vtz basis, so don't break yet
                    elif any(high_basis in line.lower() for high_basis in ['vqz', 'v5z', 'v6z', 'v7z', 'v8z']):
                        f12b = True  # MRCI could also have a v(4+)z basis, so don't break yet
+                elif 'ccsd' in line and 'f12' in line:


Use line.lower().

Also added loadScanEnergies to MolproLog for consistent inheritance

alongd · 2019-05-06T02:16:52Z

I didn't use fixup commits since I had to re-commit stuff to better organize the PR. I think I addressed everything (though I left the f12 flag, let me know if you think of a better way), and force pushed.

alongd added the Arkane label May 2, 2019

alongd requested a review from cgrambow May 2, 2019 18:17

alongd self-assigned this May 2, 2019

cgrambow requested changes May 2, 2019

View reviewed changes

alongd commented May 2, 2019

View reviewed changes

alongd force-pushed the no_f12 branch from 8be9709 to daab65a Compare May 2, 2019 20:43

cgrambow requested changes May 3, 2019

View reviewed changes

alongd added 6 commits May 5, 2019 22:05

Minor: PEP8 and style modifications to molpro.py

4bf0bf1

Minor: PEP8 and style modifications to molproTest.py

dc1e820

Minor: Removed unused frequencyScaleFactor from Arkane Log and QChemLog

8def723

Added superclass __init__ to Arkane QM Logs

acc08e8

Also added loadScanEnergies to MolproLog for consistent inheritance

Read CCSD(T) energies in MolproLog

b8d45b6

Tests: Test load CCSD(T)/aug-cc-pVQZ energy from Molpro

20ff313

alongd force-pushed the no_f12 branch from daab65a to 20ff313 Compare May 6, 2019 02:13

cgrambow approved these changes May 6, 2019

View reviewed changes

cgrambow merged commit fe57caa into master May 6, 2019

cgrambow deleted the no_f12 branch May 6, 2019 15:35

mliu49 mentioned this pull request May 15, 2019

RMG v2.4.0 (?) Release Planning #1591

Closed

5 tasks

Parse CCSD(T) energies (no F12) in Molpro #1592

Parse CCSD(T) energies (no F12) in Molpro #1592

Uh oh!

Conversation

alongd commented May 2, 2019

Uh oh!

codecov bot commented May 2, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

cgrambow left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

alongd May 2, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

alongd left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

alongd May 2, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

alongd commented May 6, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

codecov bot commented May 2, 2019 •

edited

Loading

alongd May 2, 2019 •

edited

Loading

alongd May 2, 2019 •

edited

Loading