Forbidden structs in input

[davidfarinajr]

Motivation or Problem

I wanted to add a forbidden structure to my input file, and realized that we didn't have this feature, so I am adding it here.

Description of Changes

1. add reaction system loaded from input file to reaction model (bug fix) - this is important because we need to remove forbidden species from reaction model
2. added forbidden to input.py with label and structure (mol or group)

Testing

built a few models with it, and it works as expected

[davidfarinajr]

forbidden(
    label = "CO_bidentate",
    structure = adjacencyListGroup(
    """
    1 O u0 p2 c0 {2,S} {3,S}
    2 C u0 p0 c0 {1,S} {4,[S,D,T]}
    3 X u0 p0 c0 {1,S}
    4 X u0 p0 c0 {2,[S,D,T]}
    """
    )
)

Adding species COXX(58) to model core
Species COXX(58) was Forbidden and not added to Core...Removing from Edge.

Adding species [Pt]OC=[Pt](59) to model core
Species [Pt]OC=[Pt](59) was Forbidden and not added to Core...Removing from Edge.

Adding species CO2XX(38) to model core
Species CO2XX(38) was Forbidden and not added to Core...Removing from Edge.

Adding species HOCOXX(296) to model core
Species HOCOXX(296) was Forbidden and not added to Core...Removing from Edge.

Adding species acetylXX(135) to model core
Species acetylXX(135) was Forbidden and not added to Core...Removing from Edge.

forbidden(
    label = "CO_bidentate",
    structure = adjacencyList(
    """
    1 O u0 p2 c0 {2,S} {3,S}
    2 C u0 p0 c0 {1,S} {4,T}
    3 X u0 p0 c0 {1,S}
    4 X u0 p0 c0 {2,T}
    """
    )
)

Adding CO_bidentate to the forbidden structures database
Adding species COXX(268) to model core
Species COXX(268) was Forbidden and not added to Core...Removing from Edge.

forbidden(
    label = "Ods",
    structure = adjacencyListGroup(
    """
    1 O u0 p2 c0 {2,S} {3,S}
    2 C u0 p0 c0 {1,S} {4,[S,D,T]}
    3 X u0 p0 c0 {1,S}
    4 X u0 p0 c0 {2,[S,D,T]}
    """
    )
)
rmgpy.exceptions.InputError: 
        Forbidden structure Ods label is already in the forbidden structure database.
        Please choose a different label for this structure that is not already in
        /Users/farina.d/Code/RMG-database/input/forbiddenStructures.py

[davidfarinajr]

If we like this, I can update documentation.

[codecov]

Codecov Report

Merging #2185 (1a896f1) into main (d018dc0) will decrease coverage by 0.04%.
The diff coverage is 29.41%.

@@            Coverage Diff             @@
##             main    #2185      +/-   ##
==========================================
- Coverage   47.25%   47.21%   -0.05%     
==========================================
  Files         104      104              
  Lines       27679    27695      +16     
  Branches     7120     7123       +3     
==========================================
- Hits        13080    13076       -4     
- Misses      13195    13211      +16     
- Partials     1404     1408       +4     

Impacted Files	Coverage Δ
rmgpy/rmg/main.py	21.96% <11.11%> (-0.05%)	⬇️
rmgpy/rmg/input.py	34.21% <50.00%> (+0.15%)	⬆️
rmgpy/molecule/draw.py	51.97% <0.00%> (-0.78%)	⬇️
arkane/kinetics.py	12.24% <0.00%> (ø)

---

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 d018dc0...1a896f1. Read the comment docs.

[rwest]

I've wanted this feature for a long time.

From a quick look your implementation looks reasonable to me, but I would welcome other comments.

One question I had was whether we need the separate adjacencyListGroup instead of just using adjacencyList for both (eg. if it's not a molecule, it tries interpreting as a group?) but I've not thought this through carefully.

Please add to a few example input files, including RMG-Py/examples/rmg/commented/input.py, as well as the documentation.

Edited to add:
Would something like structure=SMILES("CCC") also work? In which case, maybe include as one of the examples.

[davidfarinajr]

I've wanted this feature for a long time.

From a quick look your implementation looks reasonable to me, but I would welcome other comments.

One question I had was whether we need the separate adjacencyListGroup instead of just using adjacencyList for both (eg. if it's not a molecule, it tries interpreting as a group?) but I've not thought this through carefully.

Please add to a few example input files, including RMG-Py/examples/rmg/commented/input.py, as well as the documentation.

Edited to add:
Would something like structure=SMILES("CCC") also work? In which case, maybe include as one of the examples.

Yes, the SMILES does work with forbidden in the input. I thought a little more about the need for adjacencyListGroup. We could just use try/except with the adjacencyList function: assume it's a molecule, try that first and if KeyError or InvalidAdjacencyListError, make a group. However, since this is also used for species in the input which should only be molecules, we may mistakenly make a group if there is an error in the species adj list. Consider the example below where someone might want methane but forget an H. In this example, we would make a group because of an error in a species adj list. So its a bit annoying, but I think it is worth it to have a separate adjacencyListGroup function

[ChrisBNEU]

This is great, thanks for adding it. I agree documentation and examples should be updated. All I had was the questions below, I wasn't sure why we specifically check for the "+".

[davidfarinajr]

This is great, thanks for adding it. I agree documentation and examples should be updated. All I had was the questions below, I wasn't sure why we specifically check for the "+".

Thanks Chris. We try to avoid the "+" in the species label because "+" is the delimiter we use in a reaction label to separate reactions and products (A + B = C + D). If there is a "+" in the species label, then it will mistakenly be split if we use "+" to split the reaction label to separate the reactants and product. The forbidden species labels are not used in the reactions, so we probably could allow "+", but it's safer to just avoid using it as a general rule.

[rwest]

Thanks for adding the documentation! Made a few suggestions. Please also add to a couple of the examples at RMG-Py/examples/rmg/ (including the commented one which I think is supposed to be pretty complete)

[rwest]

Suggested change

	RMG exlores a wide varitey of structures as a builds a mechanism.
	RMG exlores a wide varitey of structures as it builds a mechanism.

[rwest]

What luck! 🤣

[rwest]

This all sounds a bit "chatty" to me. But if that's the style of the rest of the manual, it's fine. Not a big problem anyway - at least we have documentation now!

[rwest]

To "add a forbidden structure to your job" could be misinterpreted. We want to forbid a structure from our model, not add it.

[rwest]

I first thought this was an adsorbed CO molecule, and took a long time (of writing and deleting comments) before realizing it was R2C*-O*. Not sure how to fix. Maybe change its label? and/or describe it when saying "..an example of a forbidden structure.." ?

[rwest]

....and now I see you have that description just below on line 288. OK, so just move that up to before the example.

[davidfarinajr]

I added forbidden to example inputs (commented and a catalysis one)
I changed the adjlistgroup forbidden example to R-O-Cl instead (easier to interpret than C-O bidentate)

I also set spec.explicitly_allowed = True for input species that are forbidden if "input species" are allowed in generated species constraints (ee715ca). I think this is the intended behavior: If we explicitly put species in our input file and explicitly allow input species for species constraints, then we should allow them

[davidfarinajr]

Not sure why the documentation build failed https://github.com/ReactionMechanismGenerator/RMG-Py/pull/2185/checks?check_run_id=3194731015

[sevyharris]

Looks good to me!