Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Question on defaults for Troe Reaction parameters #404

Closed
skyreflectedinmirrors opened this issue Dec 5, 2016 · 5 comments
Closed

Question on defaults for Troe Reaction parameters #404

skyreflectedinmirrors opened this issue Dec 5, 2016 · 5 comments

Comments

@skyreflectedinmirrors
Copy link
Contributor

Cantera version

v2.3.0.b1

Operating System

RHEL7

Python/MATLAB version

python2.7

Expected Behavior

Undefined, see below.

Actual Behavior

Currently, on line 33-43 of Falloff.cpp the (inverse of) the Troe temperature parameters are set to 1000 in the case that the mechanism incorrectly specifies T1 or T3 to be zero.

Steps to reproduce

DME_cti.txt (note, I had to rename it to a text file to upload to github)


import cantera as ct
gas = ct.Solution('mechs/DME.cti')
print gas.reactions()[47].falloff.parameters
[  0.00000000e+00   5.70000000e+02   1.00000000e-03   1.00000000e+30]

Of note here is that T1 is set to 1e-3 (or 1 / 1000) when the mechanism has the parameter (incorrectly) set to zero. The modeler seemingly did the incorrect (but often used in engineering) simplification that:
0 * exp(-C / 0) == 0

However, this is actually undefined in a strict mathematical sense (and to boot, is crappy input)

A few questions:

  1. Are these default values significant? e.g. if they were from the original paper I would understand, but in this case we should add the defaults to the documentation
  2. Is this the desired behavior?

Generally I think we don't want a magical changing of numbers behind the scenes. Indeed in the SRI initialization we have checks that throw errors instead, e.g.:

if (c[2] < 0.0) {
        throw CanteraError("SRI::init()",
                           "m_c parameter is less than zero: {}", c[2]);
    }

If we agree that it should result in an error to specify T1 or T3 = 0, I'll submit a pull request along these lines

@skyreflectedinmirrors
Copy link
Contributor Author

Going through the original papers (thanks @bryanwweber), I don't see any of the above default values

@speth
Copy link
Member

speth commented Dec 6, 2016

I don't think there's anything particularly magical going on here, although the existing implementation may be generating a bit of unnecessary confusion.

For the T* and T*** parameters, Cantera stores their inverse as a minor optimization (so it can multiply when evaluating these terms, rather than dividing). I think when this was originally written, someone wanted to avoid division by zero, so for the case where T* or T*** was given as 0, they set the value of the inverse to be 1000, which is large enough that the exponential will underflow for any realistic value for the temperature. Of course, when returning the parameter to the user, we have to take the inverse of the stored values, which is why it gives you 0.001. For all practical purposes, this is equivalent to zero.

I don't think this aversion to division by zero is actually necessary though. The simple fix is just to remove the special case for these parameters in Falloff.cpp, which will give the expected output from the getParameters function.

I don't think we should consider setting either of these parameters to zero an error. I feel like I've seen such reactions in published mechanisms, and the rates Cantera calculates for such reactions match what Chemkin does, so they are being handled in a consistent manner.

@skyreflectedinmirrors
Copy link
Contributor Author

@speth the use of 1000 to underflow the exponential makes sense from a numerical perspective. When you refer to "removing the special case" for these parameters, what do you mean exactly? E.g. letting a division by zero happen and trapping it later?

One solution might be a suppressible warning (along the lines of the thermo discontinuities) that alerts the user that the value used is being altered.

@speth
Copy link
Member

speth commented Dec 6, 2016

I don't think there's anything that needs to be trapped. For example, if T*** is zero, then without the special case, m_rt3 will be set to 1.0/0.0 which is inf. When evaluating Fcent, we then have exp(-T*inf) = 0.0, which I believe is the desired result.

@skyreflectedinmirrors
Copy link
Contributor Author

@speth oh, didn't realize that exp(-inf) be smart enough to do that (I thought it'd go the way of NaN)! It looks like this should be the case in all modern compilers

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants