From f695b059774ed164f203a398f7852a1464f4b6d4 Mon Sep 17 00:00:00 2001 From: jonwzheng Date: Wed, 14 Feb 2024 15:32:52 -0500 Subject: [PATCH 1/5] Removed references to kinetic group additivity values from codebase We have decided to remove group additivity methods for kinetics estimation. This commit builds upon the commit from Aug. 2023 to remove additional references to kinetic estimation by group additivity, including docstring, such that the only valid kinetic estimator now is 'rate rules'. --- rmgpy/data/kinetics/family.py | 37 +++++++++++------------------------ rmgpy/rmg/main.py | 4 ++-- rmgpy/rmg/model.py | 12 ------------ 3 files changed, 13 insertions(+), 40 deletions(-) diff --git a/rmgpy/data/kinetics/family.py b/rmgpy/data/kinetics/family.py index fbb6132084..b5cda64e15 100644 --- a/rmgpy/data/kinetics/family.py +++ b/rmgpy/data/kinetics/family.py @@ -77,7 +77,7 @@ class TemplateReaction(Reaction): Attribute Type Description =============== ========================= ===================================== `family` ``str`` The kinetics family that the reaction was created from. - `estimator` ``str`` Whether the kinetics came from rate rules or group additivity. + `estimator` ``str`` The name of the kinetic estimator; currently only rate rules is supported. `reverse` :class:`TemplateReaction` The reverse reaction, for families that are their own reverse. `is_forward` ``bool`` Whether the reaction was generated in the forward direction of the family. `labeled_atoms` ``dict`` Keys are 'reactants' or 'products', values are dictionaries. @@ -2396,23 +2396,18 @@ def get_reaction_template(self, reaction): def get_kinetics_for_template(self, template, degeneracy=1, method='rate rules'): """ Return an estimate of the kinetics for a reaction with the given - `template` and reaction-path `degeneracy`. There are two possible methods - to use: 'group additivity' (new possible RMG-Py behavior) and 'rate rules' (old - RMG-Java behavior, and default RMG-Py behavior). + `template` and reaction-path `degeneracy`. There is currently only one method to use: + 'rate rules' (old RMG-Java behavior, and default RMG-Py behavior). Group additivity was removed in August 2023. Returns a tuple (kinetics, entry): If it's estimated via 'rate rules' and an exact match is found in the tree, then the entry is returned as the second element of the tuple. - But if an average is used, or the 'group additivity' method, then the tuple - returned is (kinetics, None). """ - if method.lower() == 'group additivity': - return self.estimate_kinetics_using_group_additivity(template, degeneracy), None - elif method.lower() == 'rate rules': + if method.lower() == 'rate rules': return self.estimate_kinetics_using_rate_rules(template, degeneracy) # This returns kinetics and entry data else: raise ValueError('Invalid value "{0}" for method parameter; ' - 'should be "group additivity" or "rate rules".'.format(method)) + 'currently only "rate rules" is supported.'.format(method)) def get_kinetics_from_depository(self, depository, reaction, template, degeneracy): """ @@ -2458,8 +2453,8 @@ def _select_best_kinetics(self, kinetics_list): def get_kinetics(self, reaction, template_labels, degeneracy=1, estimator='', return_all_kinetics=True): """ Return the kinetics for the given `reaction` by searching the various - depositories as well as generating a result using the user-specified `estimator` - of either 'group additivity' or 'rate rules'. Unlike + depositories as well as generating a result using the user-specified `estimator`. + Currently, only 'rate rules' is a supported estimator. Unlike the regular :meth:`get_kinetics()` method, this returns a list of results, with each result comprising of @@ -2468,7 +2463,7 @@ def get_kinetics(self, reaction, template_labels, degeneracy=1, estimator='', re 3. the entry - this will be `None` if from a template estimate 4. is_forward a boolean denoting whether the matched entry is in the same direction as the inputted reaction. This will always be True if using - rates rules or group additivity. This can be `True` or `False` if using + rates rules. This can be `True` or `False` if using a depository If return_all_kinetics==False, only the first (best?) matching kinetics is returned. @@ -2489,7 +2484,9 @@ def get_kinetics(self, reaction, template_labels, degeneracy=1, estimator='', re for kinetics, entry, is_forward in kinetics_list0: kinetics_list.append([kinetics, depository, entry, is_forward]) - # If estimator type of rate rules or group additivity is given, retrieve the kinetics. + # If estimator type of rate rules is given, retrieve the kinetics. + # TODO: Since group additivity was removed, this logic can be condensed into just 1 branch. + if estimator: try: kinetics, entry = self.get_kinetics_for_template(template, degeneracy, method=estimator) @@ -2502,7 +2499,6 @@ def get_kinetics(self, reaction, template_labels, degeneracy=1, estimator='', re return kinetics, estimator, entry, True kinetics_list.append([kinetics, estimator, entry, True]) # If no estimation method was given, prioritize rate rule estimation. - # If returning all kinetics, add estimations from both rate rules and group additivity. else: try: kinetics, entry = self.get_kinetics_for_template(template, degeneracy, method='rate rules') @@ -2513,15 +2509,6 @@ def get_kinetics(self, reaction, template_labels, degeneracy=1, estimator='', re # If kinetics were undeterminable for rate rules estimation, do nothing. pass - try: - kinetics2, entry2 = self.get_kinetics_for_template(template, degeneracy, method='group additivity') - if not return_all_kinetics: - return kinetics2, 'group additivity', entry2, True - kinetics_list.append([kinetics2, 'group additivity', entry2, True]) - except KineticsError: - # If kinetics were undeterminable for group additivity estimation, do nothing. - pass - if not return_all_kinetics: raise UndeterminableKineticsError(reaction) @@ -3599,8 +3586,6 @@ def cross_validate_old(self, folds=5, T=1000.0, random_state=1, estimator='rate template = self.retrieve_template(template_labels) if estimator == 'rate rules': kinetics, entry = self.estimate_kinetics_using_rate_rules(template, degeneracy=1) - elif estimator == 'group additivity': - kinetics = self.estimate_kinetics_using_group_additivity(template, degeneracy=1) else: raise ValueError('{0} is not a valid value for input `estimator`'.format(estimator)) diff --git a/rmgpy/rmg/main.py b/rmgpy/rmg/main.py index 8291db4a13..a78cf64af2 100644 --- a/rmgpy/rmg/main.py +++ b/rmgpy/rmg/main.py @@ -112,7 +112,7 @@ class RMG(util.Subject): `seed_mechanisms` The seed mechanisms included in the model `kinetics_families` The kinetics families to use for reaction generation `kinetics_depositories` The kinetics depositories to use for looking up kinetics in each family - `kinetics_estimator` The method to use to estimate kinetics: 'group additivity' or 'rate rules' + `kinetics_estimator` The method to use to estimate kinetics: currently, only 'rate rules' is supported `solvent` If solvation estimates are required, the name of the solvent. `liquid_volumetric_mass_transfer_coefficient_power_law` If kLA estimates are required, the coefficients for kLA power law ---------------------------------------------------------- ------------------------------------------------ @@ -183,7 +183,7 @@ def clear(self): self.seed_mechanisms = None self.kinetics_families = None self.kinetics_depositories = None - self.kinetics_estimator = "group additivity" + self.kinetics_estimator = "rate rules" self.solvent = None self.diffusion_limiter = None self.surface_site_density = None diff --git a/rmgpy/rmg/model.py b/rmgpy/rmg/model.py index 951b670d00..6e536f5ed9 100644 --- a/rmgpy/rmg/model.py +++ b/rmgpy/rmg/model.py @@ -996,18 +996,6 @@ def generate_kinetics(self, reaction): # Use the one for which the kinetics is the forward kinetics keep_reverse = gibbs_is_positive and is_forward and rev_is_forward reason = "Both directions matched the same entry in {0}, but this direction is exergonic.".format(reaction.family) - elif self.kinetics_estimator == "group additivity" and ( - kinetics.comment.find("Fitted to 1 rate") > 0 and not rev_kinetics.comment.find("Fitted to 1 rate") > 0 - ): - # forward kinetics were fitted to only 1 rate, but reverse are hopefully better - keep_reverse = True - reason = "Other direction matched a group only fitted to 1 rate." - elif self.kinetics_estimator == "group additivity" and ( - not kinetics.comment.find("Fitted to 1 rate") > 0 and rev_kinetics.comment.find("Fitted to 1 rate") > 0 - ): - # reverse kinetics were fitted to only 1 rate, but forward are hopefully better - keep_reverse = False - reason = "Other direction matched a group only fitted to 1 rate." elif entry is not None and rev_entry is not None: # Both directions matched explicit rate rules # Keep the direction with the lower (but nonzero) rank From 9ab9c40dcf2f8a088e446e05cbbe6b692c257b5a Mon Sep 17 00:00:00 2001 From: jonwzheng Date: Wed, 14 Feb 2024 15:35:49 -0500 Subject: [PATCH 2/5] Removed references of kinetic group additivity from the RMG documentation --- documentation/source/reference/data/index.rst | 2 +- documentation/source/users/rmg/kinetics.rst | 6 ++---- 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/documentation/source/reference/data/index.rst b/documentation/source/reference/data/index.rst index 9d56c1f193..2655aa99fd 100644 --- a/documentation/source/reference/data/index.rst +++ b/documentation/source/reference/data/index.rst @@ -50,7 +50,7 @@ Class Description =========================== ==================================================== :class:`DepositoryReaction` A reaction with kinetics determined from querying a kinetics depository :class:`LibraryReaction` A reaction with kinetics determined from querying a kinetics library -:class:`TemplateReaction` A reaction with kinetics determined from querying a kinetics group additivity or rate rules method +:class:`TemplateReaction` A reaction with kinetics determined from querying a rate rules method :class:`ReactionRecipe` A sequence of actions that represent the process of a chemical reaction --------------------------- ---------------------------------------------------- :class:`KineticsDepository` A depository of all kinetics parameters for one or more reactions diff --git a/documentation/source/users/rmg/kinetics.rst b/documentation/source/users/rmg/kinetics.rst index d6ca9cdeef..7ccf0b429c 100644 --- a/documentation/source/users/rmg/kinetics.rst +++ b/documentation/source/users/rmg/kinetics.rst @@ -40,11 +40,9 @@ parameters +-------+--------------------------------------------------------------------------------------+ |Rank 8 |B3LYP & lower DFT (rotors if necessary) | +-------+--------------------------------------------------------------------------------------+ -|Rank 9 |Group Additivity | +|Rank 9|Direct Estimate/Guess | +-------+--------------------------------------------------------------------------------------+ -|Rank 10|Direct Estimate/Guess | -+-------+--------------------------------------------------------------------------------------+ -|Rank 11|Average of Rates | +|Rank 10|Average of Rates | +-------+--------------------------------------------------------------------------------------+ |Rank 0 |General Estimate (Never used in generation) | +-------+--------------------------------------------------------------------------------------+ From d20749318de89c06a0c1c98bd1433b65d536deed Mon Sep 17 00:00:00 2001 From: jonwzheng Date: Wed, 14 Feb 2024 15:38:46 -0500 Subject: [PATCH 3/5] Update to get_kinetics_for_template docstring to fix accidental removal of useful description of what data type is returned --- rmgpy/data/kinetics/family.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/rmgpy/data/kinetics/family.py b/rmgpy/data/kinetics/family.py index b5cda64e15..c37eccd4df 100644 --- a/rmgpy/data/kinetics/family.py +++ b/rmgpy/data/kinetics/family.py @@ -2402,6 +2402,8 @@ def get_kinetics_for_template(self, template, degeneracy=1, method='rate rules') Returns a tuple (kinetics, entry): If it's estimated via 'rate rules' and an exact match is found in the tree, then the entry is returned as the second element of the tuple. + But if an average is used,then the tuple returned is (kinetics, None). + """ if method.lower() == 'rate rules': return self.estimate_kinetics_using_rate_rules(template, degeneracy) # This returns kinetics and entry data From 6c1d8aa4d1582d4183d4cfe54b0ba657fc79cdc2 Mon Sep 17 00:00:00 2001 From: jonwzheng Date: Fri, 16 Feb 2024 11:36:37 -0500 Subject: [PATCH 4/5] fix malformed table in kinetics documentation --- documentation/source/users/rmg/kinetics.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/documentation/source/users/rmg/kinetics.rst b/documentation/source/users/rmg/kinetics.rst index 7ccf0b429c..9db406463e 100644 --- a/documentation/source/users/rmg/kinetics.rst +++ b/documentation/source/users/rmg/kinetics.rst @@ -40,7 +40,7 @@ parameters +-------+--------------------------------------------------------------------------------------+ |Rank 8 |B3LYP & lower DFT (rotors if necessary) | +-------+--------------------------------------------------------------------------------------+ -|Rank 9|Direct Estimate/Guess | +|Rank 9 |Direct Estimate/Guess | +-------+--------------------------------------------------------------------------------------+ |Rank 10|Average of Rates | +-------+--------------------------------------------------------------------------------------+ From c0ea448eedb6a7fb882b5440cd262fd5606432ec Mon Sep 17 00:00:00 2001 From: jonwzheng Date: Fri, 16 Feb 2024 11:56:55 -0500 Subject: [PATCH 5/5] fix typo in docstring for getting kinetics from reaction template --- rmgpy/data/kinetics/family.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rmgpy/data/kinetics/family.py b/rmgpy/data/kinetics/family.py index c37eccd4df..234a85488b 100644 --- a/rmgpy/data/kinetics/family.py +++ b/rmgpy/data/kinetics/family.py @@ -2402,7 +2402,7 @@ def get_kinetics_for_template(self, template, degeneracy=1, method='rate rules') Returns a tuple (kinetics, entry): If it's estimated via 'rate rules' and an exact match is found in the tree, then the entry is returned as the second element of the tuple. - But if an average is used,then the tuple returned is (kinetics, None). + But if an average is used, then the tuple returned is (kinetics, None). """ if method.lower() == 'rate rules':