Create YAML file outputs in RMG for use in Cantera #2321

Nora-Khalil · 2022-07-13T21:30:41Z

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.

new file: 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).
changes to 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 the
surface species.
changes to rmgpy/kinetics/falloff.pyx:
- edited/addedset_cantera_kinetics() and to_cantera_kinetics() in Lindemann and Troe classes
changes to rmgpy/kinetics/surface.pyx:
- edited/added set_cantera_kinetics() and to_cantera_kinetics() in StickingCoefficient and SurfaceArrhenius
classes
changes to rmgpy/species.py:
- edited to_cantera() to account for surface sites in surface reactions.
changes to rmgpy/rmg/main.py:
- edited register_listeners(), attached CanteraWriter

Testing

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.

lgtm-com · 2022-07-13T21:56:59Z

This pull request introduces 15 alerts when merging 6574356 into 517b347 - view on LGTM.com

new alerts:

9 for Unused import
2 for Wrong name for an argument in a class instantiation
1 for Module imports itself
1 for Unused local variable
1 for Except block handles 'BaseException'
1 for Module is imported with 'import' and 'import from'

8920211a36866889.out

RMG.log

rmgpy/rmg/test_data/mainTest/RMG.profile.dot

lgtm-com · 2022-07-14T15:27:39Z

This pull request introduces 17 alerts when merging 83a052f into 517b347 - view on LGTM.com

new alerts:

11 for Unused import
2 for Wrong name for an argument in a class instantiation
1 for Module imports itself
1 for Unused local variable
1 for Except block handles 'BaseException'
1 for Module is imported with 'import' and 'import from'

rwest · 2022-07-14T16:24:43Z

I think the test failing AttributeError: 'cantera._cantera.Species' object has no attribute 'input_data' might mean that we're using an older version of Cantera? Would need to update the environment file? and warn people this is a breaking change to the requirements.

lgtm-com · 2022-07-14T16:26:55Z

This pull request introduces 17 alerts when merging 9673912 into 517b347 - view on LGTM.com

new alerts:

11 for Unused import
2 for Wrong name for an argument in a class instantiation
1 for Module imports itself
1 for Unused local variable
1 for Except block handles 'BaseException'
1 for Module is imported with 'import' and 'import from'

lgtm-com · 2022-07-14T17:05:50Z

This pull request introduces 17 alerts when merging 4544828 into 517b347 - view on LGTM.com

new alerts:

11 for Unused import
2 for Wrong name for an argument in a class instantiation
1 for Module imports itself
1 for Unused local variable
1 for Except block handles 'BaseException'
1 for Module is imported with 'import' and 'import from'

lgtm-com · 2022-07-14T19:41:58Z

This pull request introduces 17 alerts when merging e26a648 into 517b347 - view on LGTM.com

new alerts:

11 for Unused import
2 for Wrong name for an argument in a class instantiation
1 for Module imports itself
1 for Unused local variable
1 for Except block handles 'BaseException'
1 for Module is imported with 'import' and 'import from'

lgtm-com · 2022-07-14T20:34:37Z

This pull request introduces 4 alerts when merging fe9af4a into 517b347 - view on LGTM.com

new alerts:

1 for Except block handles 'BaseException'
1 for Unused import
1 for Module is imported with 'import' and 'import from'
1 for Wrong name for an argument in a class instantiation

lgtm-com · 2022-07-14T20:58:40Z

This pull request introduces 3 alerts when merging c4cff4e into 517b347 - view on LGTM.com

new alerts:

1 for Except block handles 'BaseException'
1 for Module is imported with 'import' and 'import from'
1 for Wrong name for an argument in a class instantiation

lgtm-com · 2022-07-18T19:20:30Z

This pull request introduces 1 alert when merging 7f91a01 into 517b347 - view on LGTM.com

new alerts:

1 for Except block handles 'BaseException'

lgtm-com · 2022-07-19T15:11:21Z

This pull request introduces 1 alert when merging f00c6e1 into 517b347 - view on LGTM.com

new alerts:

1 for Except block handles 'BaseException'

lgtm-com · 2022-07-19T20:33:54Z

This pull request introduces 1 alert when merging 02d84e0 into 517b347 - view on LGTM.com

new alerts:

1 for Except block handles 'BaseException'

JacksonBurns · 2023-04-14T18:59:24Z

@xiaoruiDong this PR has a merge conflict from the fix you made to the Troe model, can you resolve it?

JacksonBurns · 2023-05-12T00:36:01Z

@Nora-Khalil this PR is no longer blocked since the merging of #2417.

github-actions · 2023-08-10T08:07:15Z

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.

Nora-Khalil requested review from rwest, sevyharris and ChrisBNEU July 13, 2022 21:30

ChrisBNEU reviewed Jul 14, 2022

View reviewed changes

8920211a36866889.out Outdated Show resolved Hide resolved

ChrisBNEU reviewed Jul 14, 2022

View reviewed changes

RMG.log Outdated Show resolved Hide resolved

ChrisBNEU reviewed Jul 14, 2022

View reviewed changes

rmgpy/rmg/test_data/mainTest/RMG.profile.dot Outdated Show resolved Hide resolved

rwest force-pushed the yaml_writer_addition branch from 6574356 to 83a052f Compare July 14, 2022 15:03

rwest force-pushed the yaml_writer_addition branch 3 times, most recently from c57101b to 9673912 Compare July 14, 2022 16:03

rwest mentioned this pull request Jul 14, 2022

Updating Conda environment requirements #2322

Closed

2 tasks

ChrisBNEU mentioned this pull request Aug 18, 2022

Cantera version 2.6 #2288

Merged

ChrisBNEU assigned Nora-Khalil Feb 28, 2023

Nora-Khalil force-pushed the yaml_writer_addition branch from 02d84e0 to 3348e98 Compare March 2, 2023 14:36

JacksonBurns mentioned this pull request Mar 3, 2023

Schedule CI and Simplify Environment #2386

Closed

xiaoruiDong force-pushed the yaml_writer_addition branch from 3348e98 to 6ae5c60 Compare April 14, 2023 19:34

Nora-Khalil and others added 29 commits April 1, 2024 15:49

fixed for catalysis.

a8bc559

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

wip changes for stickingcoeffcient issue

6e1ed58

fixed "sites" v. "size" problem, yaml_writer now works for catalysis …

28f80c8

…and gas phase. Time for pull request?

Removing comments

bab0f16

Trying cantera 2.5+ from conda-forge in environment.yml

5c61b42

I suspected that the unit tests are failing because the new code can't work with cantera 2.3 from the RMG channel

Trying Cantera 2.5+ from cantera channel in environment.yml

c39cb50

Added in doc strings to methods. Removed unused imports

899ef2e

Removed unused import from reaction.py

da17893

renamed cantera.py to cantera_.py. Edited import statement in main.py…

bd23fdd

… 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

Edited high_rate, low_rate in set_cantera_kinetics() for Lindemann re…

0759ccc

…actions. Can now extract the high & low rate from the Cantera Lindemann object, which is necessary for reactionTest.py to pass successfully.

restructed set_cantera_kinetics() and to_cantera_kinetics() for Linde…

0c5a304

…mann reactions in falloff.pyx. Added Lindemann reaction checks in falloff.pyx

Minor stylish change in falloff.py/ troe to cantera

e110aab

modifying species names if there is a [ or ] in name

43bd964

additional changes to fix species names in yaml file output

6692742

rebased second time, putting new edits on top of rebase

1366709

more rebase edits, removing YAML_writer in main.py

0925dc5

Fix bug in yaml writing.

3fdb6e1

Previously the if was always True

Minor refactor for clarity.

8428b92

I think this is the same.

Rename obj_to_dict --> species_to_dict

9345a3a

Because that's what it is

Tidy up cantera yaml writer. Trying to avoid major changes.

b2677f4

Still doing things mostly the same way, just renaming variables, combining some blocks, removing X from the gas phase,

Tidying the cantera yaml writer

4ff908a

Renamed some functions and (i think) simplified some loops

Removed the extra rmgpy/rectionTest.py

43202b1

This was moved into the testing folder. I don't think any of the other changes were deliberate.

Redefine gas inside reactionTest. (needed?)

a6cf636

This had been done on a branch. Not sure if it is needed as things got moved in a confusing rebase.

Restore rmgpy/kinetics/falloff.pyx to the version on main branch.

46946a1

I think final edits to this file were mostly just accidents in rebasing, and the version on main is probably ok.

Restore rmgpy/reaction.pxd to version on main

72db235

I think this was a rebasing error.

Tidy up reaction.py, removing fix_reaction

8096877

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)

rwest force-pushed the yaml_writer_addition branch from 996780b to 68dc922 Compare April 1, 2024 19:50

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Create YAML file outputs in RMG for use in Cantera #2321

Create YAML file outputs in RMG for use in Cantera #2321

Nora-Khalil commented Jul 13, 2022 •

edited

lgtm-com bot commented Jul 13, 2022

lgtm-com bot commented Jul 14, 2022

rwest commented Jul 14, 2022

lgtm-com bot commented Jul 14, 2022

lgtm-com bot commented Jul 14, 2022

lgtm-com bot commented Jul 14, 2022

lgtm-com bot commented Jul 14, 2022

lgtm-com bot commented Jul 14, 2022

lgtm-com bot commented Jul 18, 2022

lgtm-com bot commented Jul 19, 2022

lgtm-com bot commented Jul 19, 2022

JacksonBurns commented Apr 14, 2023

JacksonBurns commented May 12, 2023

github-actions bot commented Aug 10, 2023

Create YAML file outputs in RMG for use in Cantera #2321

Are you sure you want to change the base?

Create YAML file outputs in RMG for use in Cantera #2321

Conversation

Nora-Khalil commented Jul 13, 2022 • edited

Motivation or Problem

Description of Changes

Testing

Reviewer Tips

lgtm-com bot commented Jul 13, 2022

lgtm-com bot commented Jul 14, 2022

rwest commented Jul 14, 2022

lgtm-com bot commented Jul 14, 2022

lgtm-com bot commented Jul 14, 2022

lgtm-com bot commented Jul 14, 2022

lgtm-com bot commented Jul 14, 2022

lgtm-com bot commented Jul 14, 2022

lgtm-com bot commented Jul 18, 2022

lgtm-com bot commented Jul 19, 2022

lgtm-com bot commented Jul 19, 2022

JacksonBurns commented Apr 14, 2023

JacksonBurns commented May 12, 2023

github-actions bot commented Aug 10, 2023

Nora-Khalil commented Jul 13, 2022 •

edited