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
Create YAML file outputs in RMG for use in Cantera #2321
base: main
Are you sure you want to change the base?
Conversation
This pull request introduces 15 alerts when merging 6574356 into 517b347 - view on LGTM.com new alerts:
|
6574356
to
83a052f
Compare
This pull request introduces 17 alerts when merging 83a052f into 517b347 - view on LGTM.com new alerts:
|
c57101b
to
9673912
Compare
I think the test failing |
This pull request introduces 17 alerts when merging 9673912 into 517b347 - view on LGTM.com new alerts:
|
This pull request introduces 17 alerts when merging 4544828 into 517b347 - view on LGTM.com new alerts:
|
This pull request introduces 17 alerts when merging e26a648 into 517b347 - view on LGTM.com new alerts:
|
This pull request introduces 4 alerts when merging fe9af4a into 517b347 - view on LGTM.com new alerts:
|
This pull request introduces 3 alerts when merging c4cff4e into 517b347 - view on LGTM.com new alerts:
|
This pull request introduces 1 alert when merging 7f91a01 into 517b347 - view on LGTM.com new alerts:
|
This pull request introduces 1 alert when merging f00c6e1 into 517b347 - view on LGTM.com new alerts:
|
This pull request introduces 1 alert when merging 02d84e0 into 517b347 - view on LGTM.com new alerts:
|
02d84e0
to
3348e98
Compare
@xiaoruiDong this PR has a merge conflict from the fix you made to the Troe model, can you resolve it? |
3348e98
to
6ae5c60
Compare
@Nora-Khalil this PR is no longer blocked since the merging of #2417. |
This pull request 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 pull request, otherwise it will automatically be closed in 30 days. |
Separates gas and surface reactions in the "reactions" section. Now just need to fix the bug with the C.Pt(22) stuff. Stay strong, shortie
…and gas phase. Time for pull request?
I suspected that the unit tests are failing because the new code can't work with cantera 2.3 from the RMG channel
… to reflect this name change. Edited reaction.py so depreciated Cantera term is not used. Edited reactionTest.py so that test_falloff is successful for third body reactions
…actions. Can now extract the high & low rate from the Cantera Lindemann object, which is necessary for reactionTest.py to pass successfully.
…mann reactions in falloff.pyx. Added Lindemann reaction checks in falloff.pyx
Previously the if was always True
I think this is the same.
Because that's what it is
The call rate = ct.LindemannRate(low_rate, high_rate, falloff) was causing TypeError: Argument 'rate' has incorrect type (expected cantera._cantera.Arrhenius, got cantera._cantera.ArrheniusRate) I think the ct.LindemannRate expects an Arrhenius not an ArrheniusRate. Thdis was with cantera 2.6.0 py37hb93dfd8_0 cantera running on the Ubuntu CI
Still doing things mostly the same way, just renaming variables, combining some blocks, removing X from the gas phase,
Renamed some functions and (i think) simplified some loops
The call rmgpy/reaction.py:339: in rmgpy.reaction.Reaction.to_cantera rate = ct.TroeRate( was causing TypeError: Argument 'rate' has incorrect type (expected cantera._cantera.Arrhenius, got cantera._cantera.ArrheniusRate) I think the ct.LindemannRate expects an Arrhenius not an ArrheniusRate. Thdis was with cantera 2.6.0 py37hb93dfd8_0 cantera running on the Ubuntu CI
This was moved into the testing folder. I don't think any of the other changes were deliberate.
This had been done on a branch. Not sure if it is needed as things got moved in a confusing rebase.
I think final edits to this file were mostly just accidents in rebasing, and the version on main is probably ok.
I think this was a rebasing error.
This function is only used in one place for one very specific thing so I inlined it. It also had a vague name. I then commented it out, because I'm not clear why it's needed. If it is needed, we can restore it (and put a comment as to why)
The reactionTest.py is failing. From the failure I can't see why this would be the cause, but in the idea of change-one-thing-at-a-time I'm reverting the most recent change since I think the tests USED to pass. This reverts commit a8eaa3b.
996780b
to
68dc922
Compare
Motivation or Problem
Previously, RMG created CHEMKIN files as outputs when generating a mechanism. The final CHEMKIN file for a mechanism is then converted to a CTI file, which can be used in Cantera simulations. When dealing with more complex mechanisms, RMG has a problem converting the final CHEMKIN file to a CTI file (because of numerous unmarked duplicate reactions in CHEMKIN file). To fix this issue, the CHEMKIN file has to be manually edited before file conversion can take place. This is time-consuming and tedious. Additionally, CTI input files are depreciated in Cantera 2.5 and will be removed in Cantera 3.0. For those using Cantera, it would be beneficial if RMG could directly output Cantera-formatted YAML files.
With the improvements proposed in this pull request, RMG can now directly output Cantera-formatted YAML files.
Description of Changes
These changes only work with Cantera 2.6 or greater.
RMG-Py/rmgpy/cantera.py
which creates a new Cantera-formatted YAML file each time another species is added to the mechanism, much like the CHEMKIN output files. Formats the YAML file depending on type of system (gas-phase v. catalysis).RMG-Py/rmgpy/reaction.py
:-
to_cantera()
method updated so that Cantera objects are correctly created for Troe, SurfaceArrhenius,StickingCoefficient, and Lindemann reactions.
-
fix_reaction()
method added so the equations of StickingCoefficient reactions don't include the SMILES of thesurface species.
rmgpy/kinetics/falloff.pyx
:- edited/added
set_cantera_kinetics()
andto_cantera_kinetics()
in Lindemann and Troe classesrmgpy/kinetics/surface.pyx
:- edited/added
set_cantera_kinetics()
andto_cantera_kinetics()
in StickingCoefficient and SurfaceArrheniusclasses
rmgpy/species.py
:- edited
to_cantera()
to account for surface sites in surface reactions.changes to
rmgpy/rmg/main.py
:- edited
register_listeners()
, attached CanteraWriterTesting
Tested yaml_writer_addition branch on the following examples:
-
examples/rmg/catalysis/methane_steam
-
examples/rmg/catalysis/ch4_o2
-
examples/rmg/minimal
-
examples/rmg/minimal_surface
Reviewer Tips
Please ensure this works on both gas-phase and catalysis.
These updates create yaml files of the type 'chem{#}.yaml'. The yaml files named 'chem_annotated.yaml' are not generated in this update but somewhere else in the code, so please focus review on the yaml files that are individually numbered.