-
Notifications
You must be signed in to change notification settings - Fork 46
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
Add "negative_A" option if required in cti files #68
base: main
Are you sure you want to change the base?
Conversation
If a reaction has a negative pre-exponential factor, it must have a negative_A option in the cti file. Fixes Niemeyer-Research-Group#67
pymars/soln2cti.py
Outdated
@@ -432,7 +432,10 @@ def write(solution, output_filename='', path=''): | |||
raise NotImplementedError(f'Unsupported reaction type: {type(reaction)}') | |||
|
|||
if reaction.duplicate: | |||
reaction_string += ',\n' + indent[8] + 'options = "duplicate"' | |||
if getattr(reaction, 'allow_negative_pre_exponential_factor', default=False): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove the default=
so this line is
if getattr(reaction, 'allow_negative_pre_exponential_factor', False):
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I already made that change on my local version of the solution file. I'll adjust it here on the pull request as well.
Removed the `default=False` and changed it to `False` since it gives an attribute type error.
Codecov Report
@@ Coverage Diff @@
## master #68 +/- ##
==========================================
- Coverage 66.98% 66.87% -0.11%
==========================================
Files 12 12
Lines 1266 1268 +2
Branches 294 295 +1
==========================================
Hits 848 848
- Misses 316 317 +1
- Partials 102 103 +1
Continue to review full report at Codecov.
|
That is implicit in my code, and I think it's a safe assumption: An overall net negative rate coefficient is forbidden by Cantera (and physically insane), so you must always have some duplicate reaction so the sum adds up to a positive number. |
If a reaction has a negative pre-exponential factor, it must have a negative_A option in the cti file.
Fixes #67
Suggested by @rwest
Not yet properly tested