ReactionMechanismGenerator / RMG-Py

Python version of the amazing Reaction Mechanism Generator (RMG).
http://reactionmechanismgenerator.github.io/RMG-Py/
Other
397 stars 228 forks source link

Resonance structure generation is creating radical catalyst surface sites #1820

Closed rwest closed 1 year ago

rwest commented 5 years ago

Bug Description

See https://github.com/cfgoldsmith/RMG-Py/issues/70 for further history, consequences, and debugging efforts, but the tl;dr: rmgpy.molecule.resonance.generate_allyl_delocalization_resonance_structures is creating surface site atoms (element X) with radical electrons. These then can't be found in the adsorption thermo database, because surface sites (of a metal) shouldn't have unpaired electrons.

I'm not yet sure how best to address this.

Any suggestions?

How To Reproduce

Put an assert not self.is_surface_site() in Atom.increment_radical. (eg. https://github.com/rwest/RMG-Py/commit/b6b8751ef53b0fa475ac40d4d9ae6b0c1f967419 ) then build a model with heterogeneous catalysis.

For me the resonance structure that causes the problem is *N=O

Error: multiplicity 2
1 O u0 p2 c0 {2,D}
2 N u0 p1 c0 {1,D} {3,S}
3 X u1 p0 c0 {2,S}

though I'm not sure what the form was before this structure was generated. Presumably it was something like...

Error: multiplicity 2
1 O u1 p2 c0 {2,S}
2 N u0 p1 c0 {1,S} {3,D}
3 X u0 p0 c0 {2,D}

??

Expected Behavior

It wouldn't make radical metals and, more importantly, wouldn't crash.

Installation Information

Describe your installation method and system information.

Additional Context

Another observation, perhaps related, is that for many (perhaps all?) recent runs with catalysis, a large fraction of the overall CPU time is spent on resonance structure generation (see https://github.com/cfgoldsmith/RMG-Py/issues/69#issuecomment-545692257 ). This may be just that we're making molecules with many resonance structures (as discussed in that thread), but it struck me that perhaps the resonance structure algorithms perform poorly for adsorbates. This may be a red herring though.

rwest commented 5 years ago

The previous species added to the core was

*N[O]
multiplicity 2
1 O u1 p2 c0 {2,S}
2 N u0 p1 c0 {1,S} {3,S} {4,S}
3 H u0 p0 c0 {2,S}
4 X u0 p0 c0 {2,S}

so it's quite likely that this lost an H forming

Error: multiplicity 2
1 O u1 p2 c0 {2,S}
2 N u0 p1 c0 {1,S} {3,D}
3 X u0 p0 c0 {2,D}

as hypothesized above

rwest commented 5 years ago

I tried just making it so that the action items increment_radical, decrement_radical, increment_lone_pairs, and decrement_lone_pairs do nothing to surface sites. That way a resonance structure generation that tries to put a radical on the surface will just have no effect on it, but it'll make the structure anyway (like the option "let it generate the resonance structure, then reset the radical count to 0 on the metal" above). https://github.com/rwest/RMG-Py/commit/b727616f5918f666c8c615116a0cf721cebd8ac4

That seemed to solve things and the job ran for further, but about 20 species later, it crashed with

For reaction generation 4 processes are used.
Exception in thread Thread-255:
Traceback (most recent call last):
  File "/Users/rwest/anaconda/envs/rmg3/lib/python3.7/threading.py", line 926, in _bootstrap_inner
    self.run()
  File "/Users/rwest/anaconda/envs/rmg3/lib/python3.7/threading.py", line 870, in run
    self._target(*self._args, **self._kwargs)
  File "/Users/rwest/anaconda/envs/rmg3/lib/python3.7/multiprocessing/pool.py", line 470, in _handle_results
    task = get()
  File "/Users/rwest/anaconda/envs/rmg3/lib/python3.7/multiprocessing/connection.py", line 251, in recv
    return _ForkingPickler.loads(buf.getbuffer())
  File "rmgpy/species.py", line 130, in rmgpy.species.Species.__init__
rmgpy.exceptions.SpeciesError: Multiplicities of molecules in species  do not match.

I guess there's now a species where some resonance structures have radicals (on the adsorbate) and some do not (because the unpaired electron was transferred to the metal, then disappeared). There's a check when making a new Species that the multiplicities of all its constituent molecules (resonance structures) match, which would usually make sense (so a species is either a radical or is not). This is only enforced when calling __init__, which is why we got away with it for a while, but that happens sometimes when passing things back and forth through the multiprocessing (I was generating reactions on 4 cores in parallel.

So either...

To be honest, I'm not very confident about radical adsorbates.

alongd commented 5 years ago

Some initial thoughts:

rwest commented 5 years ago

I permitted the different resonance structures of an adsorbate to have different multiplicities in https://github.com/rwest/RMG-Py/commit/d33df776aee39ed7521f8cfb6db1f6f111e52341 and it seems that nothing crashed.

github-actions[bot] commented 1 year ago

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.