Closed nyee closed 1 year ago
Suggested solution from RMG Group meeting: Repeat (or move?) forbidden structure check after we collapse reactions into species.
I haven't thought through all the details and I may be wrong, but I feel that inside a family it's better to forbid molecules (as currently) than species. If needed, add both resonance structures to the forbidden structures. This is because what we're trying to restrict is the template being applied to widely, rather than forbidding a species which shouldn't ever exist (which appears in the generic forbidden structures, not the family-specific one)
@rwest A forbidden structure in a family will not widely ban a species because you add atom labels (1, 2) to the structure. It should only ban reactions that match both the species and the reacting sites. Not 100% sure it works this way, but I think that is the intention.
In terms of solution, I know @mliu49 is making changes to how resonance structures work with matching templates, so it is probably worth waiting to see what the final result of that will be.
I think we should address this issue now, especially with the new resonance structures underway.
@mliu49 ,would you agree with the historic suggestion (above) that Repeat (or move?) forbidden structure check after we collapse reactions into species
?
I'm sad that something from a year and a half ago is now historic...
Anyway, I agree that we should address this issue. Here are some initial considerations:
Noted. If I understand correctly, this shouldn't require resonance re-generation, just checking all molecules of all species for each new reaction created (or entering the core?).
Now that we're forbidding species in forbiddenStructures.py, we no longer need to manually give all resonance structures to forbid a species. I think the original issue is solved. @mliu49, would you agree?
This issue is being automatically marked as stale because it has not received any interaction in the last 90 days. Please leave a comment if this is still a relevant issue, otherwise it will automatically be closed in 30 days.
In order to forbid a group in reaction families, you have to forbid all resonance structures. Test case we used to discover was for c1cc(C)ccc1[O] <=> c1cc[CH2]ccc1O in intra-H-migration family. I've added more inputs at bottom of post for testing.
This issue makes it very inconvenient to ban certain reactions. On the flip side, there may be a situation where only one resonance structure appears to be something we would want to forbid, but another may look more reasonable. I don't know what the correct solution is, but we should probably address this somehow...
generateReactions input:
Necessary forbidden groups: