Use adaptive OneDMin radii #131

alongd · 2019-05-09T15:57:01Z

Assuming the species is sphere-like, determine the r_min and r_max radii to use for a OneDMin L-J calculation.
Addresses #129

codecov · 2019-05-09T16:23:43Z

Codecov Report

Merging #131 into master will increase coverage by 0.49%.
The diff coverage is 74.46%.

@@            Coverage Diff             @@
##           master     #131      +/-   ##
==========================================
+ Coverage   42.63%   43.13%   +0.49%     
==========================================
  Files          27       27              
  Lines        6105     6192      +87     
  Branches     1578     1597      +19     
==========================================
+ Hits         2603     2671      +68     
- Misses       3120     3131      +11     
- Partials      382      390       +8

Impacted Files	Coverage Δ
arc/job/inputs.py	`100% <ø> (ø)`	⬆️
arc/scheduler.py	`18.92% <0%> (-0.12%)`	⬇️
arc/parser.py	`85.91% <0%> (-1.86%)`	⬇️
arc/common.py	`85.71% <100%> (+2.61%)`	⬆️
arc/__init__.py	`100% <100%> (ø)`	⬆️
arc/species/converter.py	`68.37% <33.33%> (-0.98%)`	⬇️
arc/job/job.py	`18.94% <66.66%> (+0.4%)`	⬆️
arc/species/species.py	`62.32% <88.09%> (+1.07%)`	⬆️
arc/reaction.py	`41.92% <0%> (ø)`	⬆️

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 ce32cac...b68ede6. Read the comment docs.

goldmanm · 2019-05-10T03:28:17Z

arc/species/species.py

@@ -1041,6 +1042,42 @@ def set_transport_data(self, lj_path, opt_path, bath_gas, opt_level, freq_path='
            comment=str(comment)
        )

+    def determine_radius(self):


Does the xyz used in ARC have a center of mass at 0,0,0? If not, this method can easily overestimate the radius.

I think this should be the center of geometry, not of mass, right? I.e., not weighting atom numbers in the average

I am not entirely sure what the best way to do this should be. Jasper 2014 describes $r$ as center-of-mass distance between the bath gas and molecule, so I was thinking centering around the mass would be a better metric. Though I am not convinced that the center of mass is a better metric to determine the guess for the radius. Maybe the center of atoms/geometry would be better.

goldmanm · 2019-05-10T04:03:05Z

arc/species/species.py

+                               'H2': 1.70, 'N2': 1.95, 'O2': 1.88}  # in Angstrom
+        bath_gas_r = int(math.ceil(bath_gas_radii_dict[bath_gas]))
+        species_radius = int(math.ceil(self.determine_radius()))
+        r_min = species_radius + bath_gas_r


I think this might be an overestimate, since the minimum radius could fall below this value.

Taking the max distance is likely to be an overestimate. Since we are taking the max distance in the previous one, maybe this would work better as r_max instead of r_min

goldmanm · 2019-05-10T04:11:14Z

arc/species/speciesTest.py

+        bath_gases = ['H2', 'N2', 'Ar', 'H2']
+        expected_rs = [(3, 6), (4, 7), (4, 7), (3, 6)]
+        for i, spc in enumerate([self.spc2, self.spc6, self.spc8, self.spc9]):
+            self.assertEqual(spc.determine_onedmin_radii(bath_gas=bath_gases[i]), expected_rs[i])


A more robust test might ensure that the actual radius that 1dmin would predict falls within the range arkane guesses.

Though even if it is within the range but near the edge, 1dmin can fail. That's what happened with my ibutanol peroxy, which eventually obtained a radius of 4.6 but failed when the range was 2-5.

goldmanm

I like the code structure which gives the ability to estimate the range based on the coordinates. I have some questions, which are discussed partially in the comments.

I added the isobutane peroxy radical (with the mass center manually adjusted to 0,0,0) in N2, which I have gotten to run in 1dmin giving a radius of 4.6, the test methods and ran it to see the range that the method gave (you can see it here. The method gave 6 and 9 as r_min and r_max respectively, which above the value estimated by r_min and r_max

Are the xyz's necesarilly centered around the center of mass? if not, it would be helpful to have a centering method since that is how 1dmin calculates radius.
Have the ranges presented in the test method worked when the molecules are put into 1dmin? If not, either you or I should run them to make sure they work (also see comment which could improve robustness of test)
One idea might be to set the r_max to the combined max radius of both molecules found in ARC and r_min to 3 smaller than that (with 1-4 or 2-5 being the minimum radius allowed). This might work for medium sized things like the butanol peroxy. Not sure what the maximum size might be.

alongd · 2019-05-14T01:24:34Z

Thanks for the comments!
I added fixups, let me know what you think. I plan to test it out with OneDMin before merging

goldmanm · 2019-05-14T20:26:11Z

arc/species/species.py

-        r_min = species_radius + bath_gas_r
+        bath_gas_r = bath_gas_radii_dict[bath_gas]
+        species_radius = self.determine_radius()
+        r_min = species_radius + bath_gas_r + 0.2


One question I have is why add 0.2 A to the minimum radius.

At least with the older algorithm using ibutanol peroxy as a test case, the radius given by the method appeared to be an over estimate, such that I think

r_max = max((species_radius + bath_gas_r, 4.5)) r_min = species_radius - 3

would lead to more successful calculations than the other algorithm.

I think we don't have the same definition to r_max. I think r_max is the farthest point the bath gas is at from the axis origin, so it cannot be species_radius + bath_gas_r, because for a spherical species this will put the bath gas right at the edge of the sphere, which is r_min

alongd · 2019-06-10T01:37:14Z

I removed the xyz centring function, following the e-mail conversation with Ahren. I assume that the xyz is centered around the center of mass, which is the case for ESS output.
Let me know if you have any comments, perhaps we could meet next week to discuss it.

Also added a memory attribute to OneDMin

If it is set to True in the job_types dict, and the species is not a TS and has final_xyz

alongd requested a review from goldmanm May 9, 2019 15:57

goldmanm reviewed May 10, 2019

View reviewed changes

goldmanm requested changes May 10, 2019

View reviewed changes

ReactionMechanismGenerator deleted a comment May 14, 2019

goldmanm reviewed May 14, 2019

View reviewed changes

alongd force-pushed the adaptive_1dmin branch from e86f0fd to bccd942 Compare June 10, 2019 01:02

ReactionMechanismGenerator deleted a comment Jun 10, 2019

alongd force-pushed the adaptive_1dmin branch from bccd942 to e1fcc5d Compare June 10, 2019 01:30

ReactionMechanismGenerator deleted a comment Jun 10, 2019

alongd force-pushed the adaptive_1dmin branch from e1fcc5d to a57171e Compare June 10, 2019 01:58

ReactionMechanismGenerator deleted a comment Jun 10, 2019

alongd force-pushed the adaptive_1dmin branch from ca2b954 to cd02be1 Compare June 21, 2019 15:58

ReactionMechanismGenerator deleted a comment Jun 21, 2019

ReactionMechanismGenerator deleted a comment Jun 22, 2019

alongd force-pushed the adaptive_1dmin branch from 967637f to cead3c4 Compare June 24, 2019 02:01

ReactionMechanismGenerator deleted a comment Jun 24, 2019

alongd force-pushed the adaptive_1dmin branch 2 times, most recently from d49af4c to aacb7f1 Compare June 24, 2019 13:59

ReactionMechanismGenerator deleted a comment Jun 24, 2019

alongd force-pushed the adaptive_1dmin branch 2 times, most recently from 054d8aa to 87710c1 Compare June 24, 2019 17:28

ReactionMechanismGenerator deleted a comment Jun 24, 2019

Use adaptive r_min and r_max in OneDMin

bc0da68

Also added a memory attribute to OneDMin

alongd added 12 commits June 24, 2019 14:02

Minor: Raise an error if no XYZ is passed to standardize_xyz_string

06662aa

Determine Species radius and OneDMin radii

dda24fc

Tests: Determine Species radius and radii

4ef6006

Set r_min and r_max when running a onedmin job

fea574a

Run OneDMin even if not running opt

297c9c4

If it is set to True in the job_types dict, and the species is not a TS and has final_xyz

Added a try except block to converter molecules_from_xyz

6fc2311

Import get_logger in ARC's __init__.py

6565b17

Added get_atom_radius and get_center_of_mass to common

6400ef0

Tests: atom_radius and center_of_mass in common

496a0ba

Make xyz more flexible in OneDMin calcs

3226a6f

Don't crush if path is None in parse_dipole_moment

bf97d2c

Added the OneDMin iPy notebook

b68ede6

alongd force-pushed the adaptive_1dmin branch from 87710c1 to b68ede6 Compare June 24, 2019 18:03

ReactionMechanismGenerator deleted a comment Jun 24, 2019

alongd mentioned this pull request Oct 10, 2022

How to Calculate L-J parameters in ARC #550

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use adaptive OneDMin radii #131

Use adaptive OneDMin radii #131

alongd commented May 9, 2019

codecov bot commented May 9, 2019 •

edited

goldmanm May 10, 2019

alongd May 13, 2019

goldmanm May 14, 2019

goldmanm May 10, 2019

goldmanm May 10, 2019

goldmanm May 10, 2019

goldmanm left a comment

alongd commented May 14, 2019

goldmanm May 14, 2019

goldmanm May 14, 2019

alongd Jun 10, 2019

alongd commented Jun 10, 2019

Use adaptive OneDMin radii #131

Are you sure you want to change the base?

Use adaptive OneDMin radii #131

Conversation

alongd commented May 9, 2019

codecov bot commented May 9, 2019 • edited

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

goldmanm left a comment

Choose a reason for hiding this comment

alongd commented May 14, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alongd commented Jun 10, 2019

codecov bot commented May 9, 2019 •

edited