added orca parser to arkane #1749
Conversation
arkane/orca.py
Outdated
|
|
||
| class OrcaLog(Log): | ||
| """ | ||
| Represent an output file from QChem. The attribute `path` refers to the |
arkane/orca.py
Outdated
| def load_force_constant_matrix(self): | ||
| # Orca print the hessian to .hess file. you need to provide .hess instead of .log | ||
|
|
||
| raise LogError('Could not find a successfully load Orca hessian:Parser is not inpemented') |
There was a problem hiding this comment.
Could you replace this with a more specific NotImplementedError error?
Something like: NotImplementedError('The load_force_constant_matrix method is not implemented for Orca Logs')
arkane/orca.py
Outdated
| CAUTION: The rotational entropy is not quite correctly treated here | ||
| because it includes a symmetry number that is not yet correctly | ||
| implemented in ORCA. Orca does not provide Rotational temperatures. Only E(rot) and E(trans) energies | ||
| Run gaussian to get E0 |
There was a problem hiding this comment.
Could you change the last line to `Consider using another supported software (such as Gaussian or QChem) to calculate the frequencies and derive E0.
|
Thank you for the comments I have addressed all the issues. |
|
@dranasinghe, the tests did not pass, giving the following trace: Let's talk and see how to resolve it. |
Codecov Report
@@ Coverage Diff @@
## master #1749 +/- ##
==========================================
+ Coverage 42.74% 42.92% +0.18%
==========================================
Files 81 82 +1
Lines 21078 21182 +104
Branches 5491 5519 +28
==========================================
+ Hits 9010 9093 +83
+ Misses 11067 11061 -6
- Partials 1001 1028 +27
Continue to review full report at Codecov.
|
arkane/orca.py
Outdated
| atom, coord, number, mass = [], [], [], [] | ||
|
|
||
| with open(self.path) as f: | ||
| log = f.read().splitlines() |
There was a problem hiding this comment.
Did you agree with the above comment? I didn't see a respective fix
arkane/orca.py
Outdated
| for i in reversed(range(len(log))): | ||
| line = log[i] | ||
| if 'CARTESIAN COORDINATES (ANGSTROEM)' in line: | ||
| for line in log[(i + 3):]: |
There was a problem hiding this comment.
I think the above comment was not addressed
arkane/orca.py
Outdated
| Run gaussian to get E0 | ||
| """ | ||
|
|
||
| raise LogError('Could not find a successfully load Orca scan:Parser is not inpemented') |
There was a problem hiding this comment.
I think the above comment was not addressed
arkane/orca.py
Outdated
|
|
||
| def load_energy(self, zpe_scale_factor=1.): | ||
| """ | ||
| Load the energy in J/ml from a Orca log file. Only the last energy |
There was a problem hiding this comment.
I think the above comment was not addressed
arkane/orca.py
Outdated
| zpe = float(line.split()[-4]) * constants.E_h * constants.Na | ||
| if zpe is not None: | ||
| return zpe | ||
| else: |
There was a problem hiding this comment.
I think the above comment was not addressed
arkane/orca.py
Outdated
|
|
||
| def load_scan_energies(self): | ||
|
|
||
| raise LogError('Could not find a successfully load Orca scan:Parser is not inpemented') |
There was a problem hiding this comment.
I think the above comment was not addressed
arkane/orca.py
Outdated
| if 'T1 diagnostic ' in line: | ||
| items = line.split() | ||
| return float(items[-1]) | ||
| raise ValueError('Unable to find T1 diagnostic in energy file: {}'.format(self.path)) |
There was a problem hiding this comment.
I think the above comment was not addressed
arkane/orca.py
Outdated
| atom, coord, number, mass = [], [], [], [] | ||
|
|
||
| with open(self.path) as f: | ||
| log = f.read().splitlines() |
There was a problem hiding this comment.
I didn't see any response to this comment. Could you take a look?
arkane/orca.py
Outdated
| @@ -115,30 +115,30 @@ def load_geometry(self): | |||
| for i in reversed(range(len(log))): | |||
| line = log[i] | |||
| if 'CARTESIAN COORDINATES (ANGSTROEM)' in line: | |||
| for line in log[(i + 3):]: | |||
| if not line.strip(): | |||
| for coordinates in log[(i + 2):]: | |||
There was a problem hiding this comment.
I think this should remain line - otherwise it'll be confusing for someone else to read this code, they would expect coordinates to contain actual coordinates (list or tuple), and not a string with additional info
arkane/orca.py
Outdated
|
|
||
| def load_energy(self, zpe_scale_factor=1.): | ||
| """ | ||
| Load the energy in J/ml from a Orca log file. Only the last energy | ||
| Load the energy in J/ml from an Orca log file. Only the last energy |
There was a problem hiding this comment.
Could you remove the extra space after Orca?
arkane/orca.py
Outdated
| zpe = float(line.split()[-4]) * constants.E_h * constants.Na | ||
| if zpe is not None: | ||
| return zpe | ||
| else: |
There was a problem hiding this comment.
I would change this part to:
if zpe is None:
raise LogError('Unable to find zero-point energy in Orca output file.')
return zpe
|
Thank you for the comments. I addressed all the comments. I left the comments on the original thread. |
|
@dranasinghe, take a look at the Codacy report for this branch, it suggests to add some spaces in |
arkane/orca.py
Outdated
| atom, coord, number, mass = [], [], [], [] | ||
|
|
||
| with open(self.path) as f: | ||
| log = f.read().splitlines() |
There was a problem hiding this comment.
It still shows f.read().splitlines(), not f.readlines(). Am I missing something?
arkane/orca.py
Outdated
| atom, coord, number, mass = [], [], [], [] | ||
|
|
||
| with open(self.path) as f: | ||
| log = f.read().splitlines() |
There was a problem hiding this comment.
Do you mean when you replaced it with lines = f.readlines()? Strange, we use this format frequently. What problem did you have? Would you like us to look at it together?
arkane/orca.py
Outdated
| raise LogError('The load_force_constant_matrix method is not implemented for Orca Logs') | ||
|
|
||
| def load_geometry(self): | ||
|
|
There was a problem hiding this comment.
Thank you for the comments. Once @alongd code for the translation and rotational modes get implemented I will include frequencies and rotor scans.
|
@dranasinghe, I think we're all set. Squash and rebase? |
minor: address minor issues raised by RMG developers
add OrcaLog and orca test to Arkane. currently limited to parsing energy, geometry, number of atoms, t1 diagnostic. Freq, spin multiplicity, scan, conformers are not implemented