Fix extra duplicate reactions from same template being added to mechanism

rmgpy/rmg/model.py

@@ -416,6 +416,12 @@ def check_for_existing_reaction(self, rxn):
         in the reaction database are iterated over to check if a reaction was overlooked
         (a reaction with a different "family" key as the parameter reaction).
 
+        Note, this function assumes RMG is making the mechanism through its main loop,
+        where all forward duplicates from a family are found in a single enlarge step.
+        If you artificially construct your own a core-edge model and forget to include a
+        duplicate from the same family, but then try to add the missing reaction from
+        the reverse direction, this function will find the existing forward reaction and
+        assume you've already found all the duplicates.
         """
 
         # Make sure the reactant and product lists are sorted before performing the check
@@ -443,11 +449,28 @@ def check_for_existing_reaction(self, rxn):
                 if isinstance(family_obj, KineticsLibrary) or isinstance(family_obj, KineticsFamily):
                     if not rxn.duplicate:
                         return True, rxn0
+                    elif (rxn.duplicate and rxn0.duplicate and isinstance(rxn, TemplateReaction)
+                          and isinstance(rxn0, TemplateReaction) and rxn.template is not None
+                          and rxn0.template is not None
+                          and frozenset(rxn.template) == frozenset(rxn0.template)):
+                        # Both reactions are duplicates (different templates for same species pair),
+                        # but they use the same template - so this is a true duplicate that should
+                        # not be added again
+                        return True, rxn0
                 else:
                     return True, rxn0
             elif isinstance(family_obj, KineticsFamily) and rxn_id == rxn_id0[::-1] and are_identical_species_references(rxn, rxn0):
                 if not rxn.duplicate:
                     return True, rxn0
+                elif rxn.duplicate and rxn0.duplicate:
+                    # The new reaction is a duplicate proposed from the reverse direction.
+                    # Template labels differ between forward and reverse, so template
+                    # comparison is not applicable here. Since the forward reaction is
+                    # already in the model, the reverse direction is already accounted for.
+                    # This assumes all forward duplicates have been found, which they will be
+                    # be during an RMG run, but might not be if you artificially construct your own
+                    # core edge reaction model
+                    return True, rxn0
 
         # Now check seed mechanisms
         # We want to check for duplicates in *other* seed mechanisms, but allow

test/rmgpy/rmg/modelTest.py

@@ -296,7 +296,7 @@ def setup_class(cls):
         """
         A method that is run before each unit test in this class.
         """
-        test_family = "H_Abstraction"
+        test_families = ["H_Abstraction", "intra_H_migration"]
 
         # set-up RMG object
         rmg = RMG()
@@ -308,7 +308,7 @@ def setup_class(cls):
         # kinetics family loading
         rmg.database.load_kinetics(
             os.path.join(path, "kinetics"),
-            kinetics_families=[test_family],
+            kinetics_families=test_families,
             reaction_libraries=[],
         )
         # load empty forbidden structures to avoid any dependence on forbidden structures
@@ -822,6 +822,87 @@ def test_check_for_existing_reaction_removes_duplicates_in_opposite_directions(
         assert found, "check_for_existing_reaction failed to identify existing reaction in the reverse direction"
         assert rxn == rxn_f
 
+    def test_check_for_existing_reaction_eliminates_same_reaction_from_reverse_multiple_templates(
+        self,
+    ):
+        """
+        Test that check_for_existing_reaction catches duplicate reactions
+        when there are multiple reaction sites (the model should have duplicates)
+        and the "new" reaction is equivalent to an existing one but proposed in the reverse.
+
+        Specifically, the model already contains two forward intra_H_migration reactions
+        (different templates, both marked duplicate=True).  When the same reactions are
+        subsequently generated in the reverse direction they must be recognised as already
+        present and not added a second time.
+
+        Note that the templates used here are marked as fictitious because the test database
+        is not necessarily large enough to distinguish these two cases. In any case, the
+        templates are not actually used for this test, but point to an example that can be
+        recreated with the full database.
+        """
+        cerm = CoreEdgeReactionModel()
+
+        spcA = Species().from_smiles("CC(O[O])C(C)OO")
+        spcB = Species().from_smiles("[CH2]C(OO)C(C)OO")
+        spcA.label = "CC(O[O])C(C)OO"
+        spcB.label = "[CH2]C(OO)C(C)OO"
+        # Thermo values are not used by check_for_existing_reaction; use a placeholder.
+        spcA.thermo = THERMO_DICT["O"]
+        spcB.thermo = THERMO_DICT["O"]
+
+        cerm.add_species_to_core(spcA)
+        cerm.add_species_to_core(spcB)
+
+        # Two distinct forward reactions already in the model (different templates).
+        reaction_in_model1 = TemplateReaction(
+            reactants=[spcA],
+            products=[spcB],
+            family="intra_H_migration",
+            template=["fictitious", "R4H_SSS_O(Cs)Cs", "O_rad_out", "Cs_H_out_2H"],
+            duplicate=True,
+        )
+        reaction_in_model2 = TemplateReaction(
+            reactants=[spcA],
+            products=[spcB],
+            family="intra_H_migration",
+            template=["fictitious", "R5H_SSSS_OCC_C", "O_rad_out", "Cs_H_out_2H"],
+            duplicate=True,
+        )
+
+        # The same reactions proposed in the reverse direction (different template labels,
+        # same physical transformation).
+        reaction_to_add1 = TemplateReaction(
+            reactants=[spcB],
+            products=[spcA],
+            family="intra_H_migration",
+            template=["fictitious", "R4H_SSS", "C_rad_out_2H", "O_H_out"],
+            duplicate=True,
+        )
+        reaction_to_add2 = TemplateReaction(
+            reactants=[spcB],
+            products=[spcA],
+            family="intra_H_migration",
+            template=["fictitious", "R5H_SSSS", "C_rad_out_2H", "O_H_out"],
+            duplicate=True,
+        )
+
+        cerm.add_reaction_to_core(reaction_in_model1)
+        cerm.register_reaction(reaction_in_model1)
+        cerm.add_reaction_to_core(reaction_in_model2)
+        cerm.register_reaction(reaction_in_model2)
+
+        found1, _ = cerm.check_for_existing_reaction(reaction_to_add1)
+        assert found1, (
+            "check_for_existing_reaction failed to find existing duplicate (multiple sites) "
+            "when proposed from reverse (R4H template)"
+        )
+
+        found2, _ = cerm.check_for_existing_reaction(reaction_to_add2)
+        assert found2, (
+            "check_for_existing_reaction failed to find existing duplicate (multiple sites) "
+            "when proposed from reverse (R5H template)"
+        )
+
     @classmethod
     def teardown_class(cls):
         """