Several Arkane Torsion (Rotor) related changes #2268

xiaoruiDong · 2022-02-17T01:01:32Z

Correct the Fourier fitting Procedure. A function of the following form is to be fitted.
f(x) = 1 + a cosx + b cos2x + ... + a' sinx + b' sin2x + ...
In the fitting procedure, Arkane makes an assumption that f'(0) = 0 (last line in the regression matrix), which correspond to
f'(0) = a' + 2 b' + 3 c' ....=0.
But in the past 10 years, this was set to be
f'(0) = a' + b' + c' .. = 0.
Now, it is corrected. In terms of influence, for most PES profiles, the previous mistake won't cause problems to a curve with good shape (like the PES from -CH3). For a more problematic PES profile (like the ones whose end conformed is significantly different from the initial one), the previous fitting approach may result in a tiny well with negative energies close to the origin. I haven't quantified the influence yet, but conceptually I don't think it is significant.
Enable parse scan log files with something like L 1 2 3 B. Currently, Arkane doesn't recognize this internal coordinate definition and will raise errors when parsing the file. So the 'L' defines a linear bend which can be helpful when there are ~180 angles in the molecule. Now, Arkane will simply pass the lines starting with an 'L'.
Enable Assign PES (angles vs energies) directly in the Arkane input file. Previously, Arkane can only read the profile from a QM output. The new feature introduces more flexibility when conducting scan jobs, especially when you need to combine some results together. E.g., you may have a job stop in the midway, or you want to run jobs at each scan point separately, or if you want to combine PES profiles from both forward and backward scans, now after your jobs, you can simply prepare a list of angle, and a list of energies, and fit them to the Arkane. A Unit test corresponding to this feature is also added.
Fix a minor np.linspace argument issue. [fixed in another PR, removed the commit in this PR]

xiaoruiDong · 2022-02-17T01:04:41Z

#2263 Just realize there is another fix for the linspace error

alongd

Thanks! Please see some minor comments

alongd · 2022-02-21T16:03:20Z

rmgpy/statmech/torsion.pyx

@@ -540,7 +540,7 @@ cdef class HinderedRotor(Torsion):
                A[:-1, m] = np.cos(m * angle)
                A[:-1, numterms + m - 1] = np.sin(m * angle)
            # This row forces dV/dangle = 0 at angle = 0
-            A[N, numterms:] = 1
+            A[N, numterms:] = np.arange(1., numterms)


I saw you related to this in the PR message, but could you please explain what this technically does?

The Fourier series to be fitted is

V = c0 + c1 cos(t) + c2 cos(2t) + c3 cos(3t) + ...cn cos(nt) + cn+1 sin(t) + cn+2 sin(2t) + ... + c2n sin(nt)

In RMG, it is fitted as a linear equation (Ac = V) to get the values of cs, where A is a matrix containing the value of

A = [1, cos(t), cost(2t), ..., sin(t), sint(2t)] # Each item is a column vector

However, RMG appends an extra line to the A matrix, to put an extra constraint in the fitting, which is dV/dangle = 0 at the angle of 0. If you calculate the dV/dangle, it is

dV/dangle = - c1 sin(t) - 2c2 sint(2t) - ... - ncn sin(nt) + cn+1 cos(t) + 2 cn+2 cos(2t) + .... nc2n sin(nt) # and dV/dangle (t=0) = cn+1 + 2cn+2 + ... + ncn2

So we need to append [ 0, 0, 0 ...., 1, 2, 3,..., n] to the A matrix instead of 1s.
Note, here n = numterms-1, which is due to the definition of numterms we used. numterms are the count for sin(0t), sin(1t), sin(2t), ...sin(nt) (the same for cos's), and notice sin(0t) = 0 which is not included in the A matrix.

arkane/statmech.py

alongd · 2022-02-21T16:09:57Z

arkane/statmech.py

@@ -273,6 +276,7 @@ def create_log(log_path, check_for_errors=True):
            'True': True,
            'False': False,
            'HinderedRotor': hinderedRotor,
+            'HinderedRotor1DArray': hinderedRotor1DArray,


Should we add this option to Arkane's documentation?

Good suggestion! Let me add it.

Thank you again for pointing me to the documentation. I just realize HinderedRotor may take a text file with angles and energies stored. It turns out I never find this before...So it actually does something very similar to my addition. Do you know it is still worth adding the class HinderedRotor1DArray? I can simply remove the last two commits then.

The current option only allows to take in a list of angles and energies. Your option has more arguments. Perhaps keep the current option of passing a file to ScanLog. We can make this file more machine-friendly by allowing one to pass a yaml file, where you can add all your args and a list of data points.

As suggested, I added the support of reading csv files and yaml files using ScanLog. For both method, I added unit tests and related descriptions in the documentation.

xiaoruiDong · 2022-02-25T23:51:58Z

Thanks for the review @alongd. I made several changes as suggested:

correct the argument sequence
add support of reading csv files and yaml files using ScanLog
add unit tests and documentations for all additions

alongd

Thanks! I added a few minor questions

arkane/statmech.py

alongd · 2022-02-28T08:25:27Z

arkane/statmech.py

+                energies = df[column]
+        return angle_unit, energy_unit, angles, energies
+
+    def load_yaml(self):


Do we want to allow users to specify pivots, top, and fit directly in the YAML file?

We could but, currently, this adds no benefit since we make these arguments required for HinderedRotor, as other file formats don't support reading the information from them. I decide to not add this into the scope of this PR.

xiaoruiDong · 2022-02-28T15:24:01Z

@alongd Thank you for correcting me. I have removed those leftovers. I will squash the fixup PRs and rebase them after you are satisfied.

alongd · 2022-03-05T20:39:38Z

Perfect, please go ahead and rebase

alongd · 2022-03-05T20:40:34Z

We're merging PRs even though rmg-tests fail, right? Is it being looked into?

Previously, RMG only can load PES profiles from a QM log file. Now, creating a hindered rotor from a PES profile (angles vs energies) is possible when preparing an Arkane input file. This introduces more flexibility.

xiaoruiDong · 2022-03-06T01:43:36Z

We're merging PRs even though rmg-tests fail, right? Is it being looked into?

Not entirely sure about how much it is fixed. #2271 solved some of the issues related to failures in rmg-test, if not any.

alongd · 2022-03-06T06:12:49Z

Seems like the tests fail due to an issue with RMS:
JULIA: MethodError: no method matching ReactionMechanismSimulator.Species(::String, ::Int64, ::String, ::String, ::String, ::ReactionMechanismSimulator.NASA{ReactionMechanismSimulator.EmptyThermoUncertainty}, ::Dict{Any, Any}, ::Int64, ::ReactionMechanismSimulator.StokesDiffusivity{Float64}, ::Float64, ::Int64, ::Float64)

@mjohnson541 , @hwpang , could you take a look?

I think that principally we can merge this PR, let me know if that's acceptable with the current failing rmg-tests

mjohnson541 · 2022-03-06T06:17:36Z

Yeah, we accidentally merged the RMS part of a PR that should've been a twin RMG-Py-RMS PR. Although, I'm not sure if RMG-tests was working before that either. It should be fine to merge this one without RMG-tests.

alongd

Thanks!

xiaoruiDong · 2022-03-06T20:58:24Z

@alongd @mjohnson541 Thanks everyone's help!

xiaoruiDong requested review from hwpang, alongd and mjohnson541 February 17, 2022 01:01

xiaoruiDong self-assigned this Feb 17, 2022

xiaoruiDong added Arkane Complexity: Low Type: Error Type: Feature labels Feb 17, 2022

xiaoruiDong mentioned this pull request Feb 17, 2022

Rotor scans ReactionMechanismGenerator/RMG-database#565

Merged

xiaoruiDong added the Status: Ready for Review PR is complete and ready to be reviewed label Feb 18, 2022

alongd reviewed Feb 21, 2022

View reviewed changes

xiaoruiDong added Status: WIP This is currently work-in-progress and removed Status: Ready for Review PR is complete and ready to be reviewed labels Feb 22, 2022

alongd reviewed Feb 28, 2022

View reviewed changes

xiaoruiDong force-pushed the torsion branch from 82ead91 to 5dd51b8 Compare February 28, 2022 15:13

xiaoruiDong added 7 commits March 5, 2022 20:34

Fixup: Correct the derivative calculation in 1D torsion fitting

c4bc795

Allow Arkane skips reading linear bend in Gaussian scan jobs

15bd5c9

Enable PES profile input for hindered rotor creation

085aa0c

Previously, RMG only can load PES profiles from a QM log file. Now, creating a hindered rotor from a PES profile (angles vs energies) is possible when preparing an Arkane input file. This introduces more flexibility.

Add unit test for create hindered rotor using PES profile

250644a

ScanLog support to parse yaml and csv files

26f91b7

Create unit tests for scanlog parsing files with different formats

475e85d

Update the document for HinderedRotor and HinderedRotor1DArray

bd3637a

xiaoruiDong force-pushed the torsion branch from 5dd51b8 to bd3637a Compare March 6, 2022 01:37

alongd approved these changes Mar 6, 2022

View reviewed changes

alongd merged commit 5d80ee3 into main Mar 6, 2022

alongd deleted the torsion branch March 6, 2022 06:43

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Several Arkane Torsion (Rotor) related changes #2268

Several Arkane Torsion (Rotor) related changes #2268

xiaoruiDong commented Feb 17, 2022 •

edited

Loading

xiaoruiDong commented Feb 17, 2022

alongd left a comment

alongd Feb 21, 2022

xiaoruiDong Feb 22, 2022 •

edited

Loading

alongd Feb 21, 2022

xiaoruiDong Feb 22, 2022

xiaoruiDong Feb 23, 2022 •

edited

Loading

alongd Feb 23, 2022

xiaoruiDong Feb 25, 2022

xiaoruiDong commented Feb 25, 2022

alongd left a comment

alongd Feb 28, 2022

xiaoruiDong Feb 28, 2022

xiaoruiDong commented Feb 28, 2022

alongd commented Mar 5, 2022

alongd commented Mar 5, 2022

xiaoruiDong commented Mar 6, 2022 •

edited

Loading

alongd commented Mar 6, 2022

mjohnson541 commented Mar 6, 2022

alongd left a comment

xiaoruiDong commented Mar 6, 2022

Several Arkane Torsion (Rotor) related changes #2268

Several Arkane Torsion (Rotor) related changes #2268

Conversation

xiaoruiDong commented Feb 17, 2022 • edited Loading

xiaoruiDong commented Feb 17, 2022

alongd left a comment

Choose a reason for hiding this comment

alongd Feb 21, 2022

Choose a reason for hiding this comment

xiaoruiDong Feb 22, 2022 • edited Loading

Choose a reason for hiding this comment

alongd Feb 21, 2022

Choose a reason for hiding this comment

xiaoruiDong Feb 22, 2022

Choose a reason for hiding this comment

xiaoruiDong Feb 23, 2022 • edited Loading

Choose a reason for hiding this comment

alongd Feb 23, 2022

Choose a reason for hiding this comment

xiaoruiDong Feb 25, 2022

Choose a reason for hiding this comment

xiaoruiDong commented Feb 25, 2022

alongd left a comment

Choose a reason for hiding this comment

alongd Feb 28, 2022

Choose a reason for hiding this comment

xiaoruiDong Feb 28, 2022

Choose a reason for hiding this comment

xiaoruiDong commented Feb 28, 2022

alongd commented Mar 5, 2022

alongd commented Mar 5, 2022

xiaoruiDong commented Mar 6, 2022 • edited Loading

alongd commented Mar 6, 2022

mjohnson541 commented Mar 6, 2022

alongd left a comment

Choose a reason for hiding this comment

xiaoruiDong commented Mar 6, 2022

xiaoruiDong commented Feb 17, 2022 •

edited

Loading

xiaoruiDong Feb 22, 2022 •

edited

Loading

xiaoruiDong Feb 23, 2022 •

edited

Loading

xiaoruiDong commented Mar 6, 2022 •

edited

Loading