From 8a031f2c7f0ec6d6a44adc4b39ae58d32e035e3d Mon Sep 17 00:00:00 2001 From: Richard West Date: Fri, 11 Aug 2023 14:10:04 -0400 Subject: [PATCH 1/6] Chemkin read: Tweak error logging. Report the reaction line. Makes debugging easier if it reports the full reaction string. (At this point in the code 'reaction' is partially read) --- rmgpy/chemkin.pyx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rmgpy/chemkin.pyx b/rmgpy/chemkin.pyx index 1988419057d..bfcbce015c1 100644 --- a/rmgpy/chemkin.pyx +++ b/rmgpy/chemkin.pyx @@ -397,7 +397,7 @@ def _read_kinetics_reaction(line, species_dict, Aunits, Aunits_surf, Eunits): # this identifies reactions like 'H+H+M=H2+M' as opposed to 'H+H(+M)=H2(+M)' as identified above third_body = True elif reactant not in species_dict: - raise ChemkinError('Unexpected reactant "{0}" in reaction {1}.'.format(reactant, reaction)) + raise ChemkinError('Unexpected reactant "{0}" in reaction line {1}.'.format(reactant, line)) else: reactant_species = species_dict[reactant] if not reactant_species.reactive: From 077aa1413b0ebe5149be2dab085963755c28590a Mon Sep 17 00:00:00 2001 From: Richard West Date: Fri, 11 Aug 2023 14:11:11 -0400 Subject: [PATCH 2/6] Quantity: just adding comments. No substantive changes. --- rmgpy/quantity.py | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/rmgpy/quantity.py b/rmgpy/quantity.py index 5d73546d455..877e44b748a 100644 --- a/rmgpy/quantity.py +++ b/rmgpy/quantity.py @@ -840,24 +840,23 @@ def RateCoefficient(*args, **kwargs): # SurfaceRateCoefficient is handled as a special case since it can take various # units depending on the reaction order SURFACERATECOEFFICIENT_CONVERSION_FACTORS = { - (1.0 / pq.s).dimensionality: 1.0, - (pq.m ** 3 / pq.s).dimensionality: 1.0, + (1.0 / pq.s).dimensionality: 1.0, # unimolecular + (pq.m ** 3 / pq.s).dimensionality: 1.0, # single site adsorption (pq.m ** 6 / pq.s).dimensionality: 1.0, (pq.m ** 9 / pq.s).dimensionality: 1.0, - (pq.m ** 3 / (pq.mol * pq.s)).dimensionality: 1.0, + (pq.m ** 3 / (pq.mol * pq.s)).dimensionality: 1.0, # single site adsorption (pq.m ** 6 / (pq.mol ** 2 * pq.s)).dimensionality: 1.0, (pq.m ** 9 / (pq.mol ** 3 * pq.s)).dimensionality: 1.0, - (pq.m ** 2 / pq.s).dimensionality: 1.0, - (pq.m ** 5 / pq.s).dimensionality: 1.0, - (pq.m ** 2 / (pq.mol * pq.s)).dimensionality: 1.0, - (pq.m ** 5 / (pq.mol ** 2 * pq.s)).dimensionality: 1.0, + (pq.m ** 2 / pq.s).dimensionality: 1.0, # bimolecular surface (Langmuir-Hinshelwood) + (pq.m ** 5 / pq.s).dimensionality: 1.0, # dissociative adsorption + (pq.m ** 2 / (pq.mol * pq.s)).dimensionality: 1.0, # bimolecular surface (Langmuir-Hinshelwood) + (pq.m ** 5 / (pq.mol ** 2 * pq.s)).dimensionality: 1.0, # dissociative adsorption (pq.m ** 4 / (pq.mol ** 2 * pq.s)).dimensionality: 1.0, } SURFACERATECOEFFICIENT_COMMON_UNITS = [ 's^-1', # unimolecular 'm^3/(mol*s)', 'cm^3/(mol*s)', 'm^3/(molecule*s)', 'cm^3/(molecule*s)', # single site adsorption - 'm^2/(mol*s)', 'cm^2/(mol*s)', 'm^2/(molecule*s)', 'cm^2/(molecule*s)', - # bimolecular surface (Langmuir-Hinshelwood) + 'm^2/(mol*s)', 'cm^2/(mol*s)', 'm^2/(molecule*s)', 'cm^2/(molecule*s)', # bimolecular surface (Langmuir-Hinshelwood) 'm^5/(mol^2*s)', 'cm^5/(mol^2*s)', 'm^5/(molecule^2*s)', 'cm^5/(molecule^2*s)', # dissociative adsorption 'm^4/(mol^2*s)', 'cm^4/(mol^2*s)', 'm^4/(molecule^2*s)', 'cm^4/(molecule^2*s)', # Surface_Bidentate_Dissociation ] From 61fd00d49137663c4babda5080ab90e3381376b0 Mon Sep 17 00:00:00 2001 From: Richard West Date: Fri, 11 Aug 2023 14:12:20 -0400 Subject: [PATCH 3/6] Chemkin read: cope with reactions with 4 reactants It needs fifth order units to be specified also to prepare for pressure-dependent reactions. --- rmgpy/chemkin.pyx | 2 ++ 1 file changed, 2 insertions(+) diff --git a/rmgpy/chemkin.pyx b/rmgpy/chemkin.pyx index bfcbce015c1..761409eaebb 100644 --- a/rmgpy/chemkin.pyx +++ b/rmgpy/chemkin.pyx @@ -1405,6 +1405,7 @@ def read_reactions_block(f, species_dict, read_comments=True): '{0}^3/({1}*{2})'.format(volume_units, molecule_units, time_units), # Second-order '{0}^6/({1}^2*{2})'.format(volume_units, molecule_units, time_units), # Third-order '{0}^9/({1}^3*{2})'.format(volume_units, molecule_units, time_units), # Fourth-order + f"{volume_units}^12/({molecule_units}^4*{time_units})", # Fifth-order ] Aunits_surf = [ @@ -1412,6 +1413,7 @@ def read_reactions_block(f, species_dict, read_comments=True): 's^-1'.format(time_units), # First-order '{0}^2/({1}*{2})'.format(area_units, molecule_units, time_units), # Second-order '{0}^4/({1}^2*{2})'.format(area_units, molecule_units, time_units), # Third-order + '{0}^6/({1}^3*{2})'.format(area_units, molecule_units, time_units), # Fourth-order ] Eunits = energy_units From f84564476dc8b3f32b94ccb769c9e654ff9093b1 Mon Sep 17 00:00:00 2001 From: Richard West Date: Fri, 11 Aug 2023 14:13:44 -0400 Subject: [PATCH 4/6] Chemkin-read: skip reactions with unit errors. The assumed units for a fourth order surface reaction are not currently allowed by the Quantities package for a SurfaceRateCoefficient expression so it raises a QuantityError. This will now just skip the reaction. --- rmgpy/chemkin.pyx | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/rmgpy/chemkin.pyx b/rmgpy/chemkin.pyx index 761409eaebb..d3b6aadea8e 100644 --- a/rmgpy/chemkin.pyx +++ b/rmgpy/chemkin.pyx @@ -47,7 +47,7 @@ from rmgpy.data.kinetics.library import LibraryReaction from rmgpy.exceptions import ChemkinError from rmgpy.molecule.element import get_element from rmgpy.molecule.util import get_element_count -from rmgpy.quantity import Quantity +from rmgpy.quantity import Quantity, QuantityError from rmgpy.reaction import Reaction from rmgpy.rmg.pdep import PDepNetwork, PDepReaction from rmgpy.species import Species @@ -1487,6 +1487,9 @@ def read_reactions_block(f, species_dict, read_comments=True): continue else: raise + except QuantityError as e: + logging.warning(f"Skipping the reaction {kinetics!r} due to units error: {e}") + continue reaction_list.append(reaction) return reaction_list From b9d35a28157ad82226d0651817f30def655e62ba Mon Sep 17 00:00:00 2001 From: Richard West Date: Fri, 11 Aug 2023 23:21:38 -0400 Subject: [PATCH 5/6] Raise ValueError if trying to match a template with wrong number of reactants. In a regular RMG run this would never happen, but sometimes other tools could be, for example, trying to see if a reaction matches a certain family, and they might ask "can you match this template?", so it's helpful to return a sensible error that they can handle. --- rmgpy/data/kinetics/family.py | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/rmgpy/data/kinetics/family.py b/rmgpy/data/kinetics/family.py index eb7223836e9..4fa3906c2f8 100644 --- a/rmgpy/data/kinetics/family.py +++ b/rmgpy/data/kinetics/family.py @@ -2709,12 +2709,16 @@ def get_labeled_reactants_and_products(self, reactants, products, relabel_atoms= template_reactants = [x.item for x in template.reactants] if len(reactants0) == 1: + if len(template_reactants) != 1: + raise ValueError(f"Reaction has {len(reactants0)} reactants but template has {len(template_reactants)} reactants.") molecule = reactants0[0] mappings = self._match_reactant_to_template(molecule, template_reactants[0]) mappings = [[map0] for map0 in mappings] num_mappings = len(mappings) reactant_structures = [molecule] elif len(reactants0) == 2: + if len(template_reactants) != 2: + raise ValueError(f"Reaction has {len(reactants0)} reactants but template has {len(template_reactants)} reactants.") molecule_a = reactants0[0] molecule_b = reactants0[1] # get mappings in forward direction @@ -2729,6 +2733,8 @@ def get_labeled_reactants_and_products(self, reactants, products, relabel_atoms= reactant_structures = [molecule_a, molecule_b] num_mappings = len(mappings_a) * len(mappings_b) elif len(reactants0) == 3: + if len(template_reactants) != 3: + raise ValueError(f"Reaction has {len(reactants0)} reactants but template has {len(template_reactants)} reactants.") molecule_a = reactants0[0] molecule_b = reactants0[1] molecule_c = reactants0[2] @@ -2743,7 +2749,7 @@ def get_labeled_reactants_and_products(self, reactants, products, relabel_atoms= reactant_structures = [molecule_a, molecule_b, molecule_c] num_mappings = len(mappings_a) * len(mappings_b) * len(mappings_c) else: - raise IndexError('You have {0} reactants, which is unexpected!'.format(len(reactants))) + raise NotImplementedError('You have {0} reactants, which is unexpected!'.format(len(reactants))) for mapping in mappings: try: From 542d196046c6545e65a78a17a3daf1691894914d Mon Sep 17 00:00:00 2001 From: Richard West Date: Tue, 15 Aug 2023 11:08:56 -0400 Subject: [PATCH 6/6] (Minor) Change an error message formatting to f-string. --- rmgpy/data/kinetics/rules.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/rmgpy/data/kinetics/rules.py b/rmgpy/data/kinetics/rules.py index 7a71070937b..7dfc5424c19 100644 --- a/rmgpy/data/kinetics/rules.py +++ b/rmgpy/data/kinetics/rules.py @@ -449,8 +449,8 @@ def estimate_kinetics(self, template, degeneracy=1): kinetics_list = remove_identical_kinetics(saved_kinetics) if len(kinetics_list) == 0: - raise KineticsError('Unable to determine kinetics for reaction with template {0} in family ' - '{1}.'.format(template, self.label)) + raise KineticsError("Unable to determine kinetics for reaction " + f"with template {template!r} in family {self.label!s}.") elif len(kinetics_list) == 1: kinetics, t = kinetics_list[0]