Modifications to Arrhenius BM's get_w0() to enable halocarbene_recombination_double retraining

[Nora-Khalil]

Motivation or Problem

The halocarbene_recombination_double family recipe is written as follows:

recipe(actions=[
    ['LOSE_PAIR', '*1', '1'],
    ['LOSE_PAIR', '*2', '1'],
    ['FORM_BOND', '*1', 1, '*2'],
    ['CHANGE_BOND', '*1', 1, '*2']
])

where there is no bond between atoms *1 and *2 in the reactants, and a double bond between *1 and *2 in the product.

This causes an error when using these reactions to fit a ArrheniusBM. When get_w0() is called for calculating the w0 for the BM's kinetics, a ValueError is raised:

ValueError: The specified vertices are not connected by an edge in this graph

This is because the current code in get_w0 will attempt to calculate the total bond energy of bonds formed (wf) by first finding the difference between the bond dissociation energy of bond between *1 and *2 in the reactants and the bond between *1 and *2 in the product. The code doesn't seem to expect that there would be no bond between *1 and *2 in the reactants if the reaction recipe has a CHANGE_BOND step.

I iterated through all families in 'default' looking for recipes that form a bond between two atoms and then change the bond (i.e. going from no bond in reactants to double or higher bond in product). I could only find halocarbene_recombination_double that has this, so I believe this PR fix is only needed for the retraining of this family.

Description of Changes

Implemented a try/except that catches ValueError, confirms that it is an example of the problem case described above, then creates a Bond object with an order of 0 (so code downstream won't crash) and a bond dissociation energy of 0.0 so the calculation for wf is still accurate.

Was able to successfully retrain family with this fix.

Testing

Here's some simple code for testing...

Before PR, ValueError would be caught in following code snippet. After PR, should pass:

fam_name = 'halocarbene_recombination_double'
training_reactions = [ent.item for ent in database.kinetics.families[fam_name].get_training_depository().entries.values()]
for tr in training_reactions: 
    try:   
        print(tr)
        display(tr)
        get_w0(database.kinetics.families[fam_name].forward_recipe.actions, tr)
        print('passed')
    except ValueError as e: 
        print(e)
        continue

Thanks!

[sevyharris]

Hey, sorry to be super pedantic, but you should run a linter to highlight the PEP8 violations (mostly rules for spacing, I recommend the Flake8 extension in VSCode). I know, the rest of the file isn't compliant either, but the new stuff adds a bunch of angry red squigglies in my code editor.

The two main rule violations I'm seeing are that 1. your comments are supposed to have at least one space after the '#' and 2. operators like == should have spaces on either side.

[sevyharris]

I think you should add a corresponding "else"

if there's a ValueError and it's not the specific case this is designed to catch, you should re-raise the value error. I think the command is just raise e

[Nora-Khalil]

Just added!

[sevyharris]

I tested this fix on halocarbene_recombination_double and can confirm that it fixes the ValueError problem without changing the binding energies in any of the other families' training reactions (that can call this function without errors).

I will note that I was also able to get ValueError: The specified vertices are not connected by an edge in this graph. for '1,4_Cyclic_birad_scission' and 'Singlet_Carbene_Intra_Disproportionation' training reactions. This PR doesn't fix those, and I don't think it has to. Just something to be aware of.

Anyways, this looks good to me and I'll approve it once the above suggestions are in.

[Nora-Khalil]

Hi @sevyharris just added in a statement that raises e if the ValueError is not one we expected. Thanks for the suggestion!

And I'll look into the other families that also get the ValueError...but later :)

Thanks!