RMG-Py icon indicating copy to clipboard operation
RMG-Py copied to clipboard

Solution phase models might include PDep rates from libraries

Open alongd opened this issue 6 years ago • 4 comments

Bug Description

If a kinetic library has pressure-dependent rates, and this library is used in a simulation involving a LiquidReactor, some of these rates end up in the core, making RMG crash when it checks the model:

  File "/home/alongd/Code/RMG-Py/rmgpy/rmg/main.py", line 982, in execute
    self.check_model()
  File "/home/alongd/Code/RMG-Py/rmgpy/rmg/main.py", line 1202, in check_model
    p_min=self.Pmin, p_max=self.Pmax)
  File "rmgpy/reaction.py", line 1217, in rmgpy.reaction.Reaction.check_collision_limit_violation
  File "rmgpy/reaction.py", line 1250, in rmgpy.reaction.Reaction.check_collision_limit_violation
  File "rmgpy/kinetics/arrhenius.pyx", line 775, in rmgpy.kinetics.arrhenius.PDepArrhenius.get_rate_coefficient
  File "rmgpy/kinetics/arrhenius.pyx", line 785, in rmgpy.kinetics.arrhenius.PDepArrhenius.get_rate_coefficient
ValueError: No pressure specified to pressure-dependent PDepArrhenius.get_rate_coefficient().

Although this is a conflict in the user's definitions (use LiquidReactor and use a library with PDep expressions), I still think this is a bug, since we don't warn not to do so, and RMG should have caught that on time, either crashing with a helpful error message or dealing with it appropriately (see suggestion below).

How To Reproduce

Run a LiquidReactor simulation with the NOx2018 kinetic library including methanol in the initial species (so that OH + CH3 = CH3OH, for example, is taken from that library), or with one of the BurkeH2O2 kinetic libraries with H2 as a core species.

Expected Behavior

I would expect RMG to at least try taking the high pressure limit rate from a pdep expression, if available (we already have the functionality to do that: https://github.com/ReactionMechanismGenerator/RMG-Py/blob/master/rmgpy/data/kinetics/library.py#L150). If unavailable, we need to decide whether to crash with a helpful message, or warn the user while ignoring that library reaction.

alongd avatar Jan 20 '20 00:01 alongd

So current LiquidReactor does have a P attribute that defaults to P=1e8 Pa to get reactions to the high pressure limit. This error is actually implicitly caused by line 677 in main.py because LiquidReactor doesn't have Prange attribute.

        try:
            self.Pmin = min([x.Prange[0].value_si if x.Prange else x.P.value_si for x in self.reaction_systems])
            self.Pmax = max([x.Prange[1].value_si if x.Prange else x.P.value_si for x in self.reaction_systems])
        except AttributeError:
            # For LiquidReactor, Pmin and Pmax remain with the default value of `None`
            pass

This can be simply solved by adding a Prange attribute default to None in LiquidReactor. I will have a PR for this shortly.

hwpang avatar Jul 29 '20 16:07 hwpang

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.

github-actions[bot] avatar Jun 21 '23 22:06 github-actions[bot]

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.

github-actions[bot] avatar Sep 22 '23 08:09 github-actions[bot]

This is a simple bug, which should be fixed. Adding to project board.

JacksonBurns avatar Sep 22 '23 14:09 JacksonBurns