Add regression test for RMS reactor #2446

hwpang · 2023-05-24T15:47:55Z

Motivation or Problem

Currently we don't have any regression test for RMS based reactors. See #2442.

Description of Changes

I add input.py and regression_input.py to test RMS's Constant V Ideal Gas Reactor and RMS's CSTR. Once this regression runs normally on CI, I will add more for other RMS reactors.

JacksonBurns · 2023-05-24T16:36:40Z

This PR resolves #2442

hwpang · 2023-05-24T17:08:01Z

One strange thing I found when making these regression tests: regardless of what reactors are used to generate the mechanism, we are using Cantera's IdealGasReactor to simulate the generated mechanism in the regression step. This includes the liquid oxidation regression test: https://github.com/ReactionMechanismGenerator/RMG-Py/blob/main/test/regression/liquid_oxidation/regression_input.py#L17. This makes sense if we just want to compare the generated mechanism and use IdealGasReactor as a standardized simulator. Although it makes the regression test for catalytic cases a bit awkward.

hwpang · 2023-05-24T17:31:51Z

I added regression tests for three RMS reactors: constant V ideal gas reactor, CSTR through constant T V liquid reactor with inlet and outlet, and liquid surface reactor.

The constant V ideal gas reactor regression test uses an input file based on superminial example.
The CSTR regression test uses an input file based on liquid_oxidation regression test, where I remove the constant assumption for oxygen, and instead flow oxygen into the reactor at a constant composition.
The liquid surface regression test uses an input file based on the catalysis/ch4o2cat example, while I use pentane as a placeholder for solvation correction. I am not familiar with what solvents are more commonly used in the catalysis system. I would greatly appreciate any more realistic settings (concentrations, temperature, solvent, etc) to use for the test.

rwest · 2023-05-24T21:35:25Z

regardless of what reactors are used to generate the mechanism, we are using Cantera's IdealGasReactor to simulate in the regression step

This does sound curious. Hard to change?

hwpang · 2023-05-24T22:44:24Z

regardless of what reactors are used to generate the mechanism, we are using Cantera's IdealGasReactor to simulate in the regression step

This does sound curious. Hard to change?

Currently, we hard coded to use Cantera class for simulation here.

Switching to use the reactor that we generate the mechanism shouldn't be too hard. We just need to add a class to simulate RMG-based reactor and a class to simulate for RMS-based reactor and the downstream process to return the organized data same as the Cantera class's simulate function

To simulate RMG-based reactor, you could probably borrow the code in the simulation script.
To simulate RMS-based reactors, you really just need to make the reactor (for example, for constant V ideal gas reactor, you can use the readily available code here to make the reactor easily) and just call solve.

hwpang · 2023-05-26T14:53:40Z

The regression test passed for the constant V ideal gas reactor and the CSTR ones, but it failed for the liquid-surface reactor. It failed because we currently don't have a updatefilterthresholds! function for SystemSimulation, which handles multi-domain case. We currently only have one for Simulation, which handles single-domain case.

We could add one for the multi-domain case in the future. Although I take a quick look at the version for Simulation, I don't see anything specific for Simulation. Any comments on this, Matt? @mjohnson541

I don't think this is something needs to be resolved with this PR. I'm going to remove the regression test for the explicit two-phase liquid-surface reactor.

hwpang · 2023-05-26T14:58:48Z

I rebased and removed the regression test for RMS's liquidSurfaceReactor explicitly

mjohnson541 · 2023-05-26T19:28:58Z

You can turn off the filter and that should let that RMG job run properly. The barrier to turning the filter on for SystemSimulations is that filter rate constants are only defined between liquid species and between gas phase species. I don't believe we have defined filter rate coefficients on surfaces or on inter-phase reactions. We should be able to turn it on for SystemSimulations that involve the same phase...which the code right now would prevent.

hwpang · 2023-05-26T20:27:07Z

@mjohnson541 Good point. I turned off the filtering for the regression test case for the liquid surface reactor in the new push.

hwpang · 2023-05-30T15:17:59Z

The CI failed at the point expected by @JacksonBurns and I. I think this PR is now ready to be reviewed. I will rebase the branch with main and turn this from a WIP to ready to review.

JacksonBurns

The CI fail is to be expected here as @hwpang stated. Commenting on this but not approving since I can't critique the new examples.

mjohnson541

These changes look good to me...although some commits should be squashed.

hwpang · 2023-06-05T15:16:00Z

I squashed relevant commits and rebased! I will force merge once the tests proceed to the expected failure point.

JacksonBurns · 2023-06-05T22:50:16Z

This is blocked by an issue with the CI (#2455) and the attempted resolution #2456

…imal

saveEdgeSpecies should be in options block

…talyte example using pentane as solvent placeholder Decrease tolerance for ch4o2cat to include more species and reactions Update model settings borrowed from liquid oxidation Remove filtering for liquid surface reactor as we currently don't have a filtering threshold for surface reactions

…actor

hwpang · 2023-06-07T18:50:34Z

I rebased this PR with main.

hwpang · 2023-06-08T00:16:09Z

The CI test failed at the expected point as there's no "stable" version of the regression tests I just added. I think this PR is ready to merge.

JacksonBurns

Huge thanks @hwpang ! LGTM

rwest · 2023-06-09T13:25:24Z

Unfortunately there seems to be some sort of problem when the CI is trying to compare the results of Regression test RMS_liquidSurface_ch4o2cat:

see
https://github.com/ReactionMechanismGenerator/RMG-Py/actions/runs/5216023343/jobs/9424779686?pr=2452
https://github.com/ReactionMechanismGenerator/RMG-Py/actions/runs/5216023343?pr=2452 for summary, and artifacts (with logs).

Traceback (most recent call last):
  File "/usr/share/miniconda3/envs/rmg_env/lib/python3.7/site-packages/julia/pseudo_python_cli.py", line 308, in main
    python(**vars(ns))
  File "/usr/share/miniconda3/envs/rmg_env/lib/python3.7/site-packages/julia/pseudo_python_cli.py", line 59, in python
    scope = runpy.run_path(script, run_name="__main__")
  File "/usr/share/miniconda3/envs/rmg_env/lib/python3.7/runpy.py", line 263, in run_path
    pkg_name=pkg_name, script_name=fname)
  File "/usr/share/miniconda3/envs/rmg_env/lib/python3.7/runpy.py", line 96, in _run_module_code
    mod_name, mod_spec, pkg_name, script_name)
  File "/usr/share/miniconda3/envs/rmg_env/lib/python3.7/runpy.py", line 85, in _run_code
    exec(code, run_globals)
  File "scripts/checkModels.py", line 294, in <module>
    error = main()
  File "scripts/checkModels.py", line 78, in main
    error = check(name, bench_chemkin, bench_species_dict, test_chemkin, test_species_dict)
  File "scripts/checkModels.py", line 92, in check
    execute(benchChemkin, benchSpeciesDict, bench_thermo, testChemkin, testSpeciesDict, test_thermo, **kwargs)
  File "/home/runner/work/RMG-Py/RMG-Py/rmgpy/tools/diffmodels.py", line 321, in execute
    model1.species, model1.reactions = load_chemkin_file(chemkin1, species_dict1, thermo_path=thermo1)
  File "rmgpy/chemkin.pyx", line 951, in rmgpy.chemkin.load_chemkin_file
    with open(path, 'r') as f:
FileNotFoundError: [Errno 2] No such file or directory: 'stable_regression_results/RMS_liquidSurface_ch4o2cat/chemkin/chem_annotated.inp'
RMS_liquidSurface_ch4o2cat Failed Core Comparison
Traceback (most recent call last):
  File "/usr/share/miniconda3/envs/rmg_env/lib/python3.7/site-packages/julia/pseudo_python_cli.py", line 308, in main
    python(**vars(ns))
  File "/usr/share/miniconda3/envs/rmg_env/lib/python3.7/site-packages/julia/pseudo_python_cli.py", line 59, in python
    scope = runpy.run_path(script, run_name="__main__")
  File "/usr/share/miniconda3/envs/rmg_env/lib/python3.7/runpy.py", line 263, in run_path
    pkg_name=pkg_name, script_name=fname)
  File "/usr/share/miniconda3/envs/rmg_env/lib/python3.7/runpy.py", line 96, in _run_module_code
    mod_name, mod_spec, pkg_name, script_name)
  File "/usr/share/miniconda3/envs/rmg_env/lib/python3.7/runpy.py", line 85, in _run_code
    exec(code, run_globals)
  File "scripts/checkModels.py", line 294, in <module>
    error = main()
  File "scripts/checkModels.py", line 78, in main
    error = check(name, bench_chemkin, bench_species_dict, test_chemkin, test_species_dict)
  File "scripts/checkModels.py", line 92, in check
    execute(benchChemkin, benchSpeciesDict, bench_thermo, testChemkin, testSpeciesDict, test_thermo, **kwargs)
  File "/home/runner/work/RMG-Py/RMG-Py/rmgpy/tools/diffmodels.py", line 321, in execute
    model1.species, model1.reactions = load_chemkin_file(chemkin1, species_dict1, thermo_path=thermo1)
  File "rmgpy/chemkin.pyx", line 951, in rmgpy.chemkin.load_chemkin_file
    with open(path, 'r') as f:
FileNotFoundError: [Errno 2] No such file or directory: 'stable_regression_results/RMS_liquidSurface_ch4o2cat/chemkin/chem_edge_annotated.inp'
RMS_liquidSurface_ch4o2cat Failed Edge Comparison
ERROR:root:The input file "/home/runner/work/RMG-Py/RMG-Py/test/regression/RMS_liquidSurface_ch4o2cat/regression_input.py" was invalid:
ERROR:root:species() got an unexpected keyword argument 'reactive'
Traceback (most recent call last):
  File "rmgpy/tools/regression.py", line 85, in read_input_file
    exec(f.read(), global_context, local_context)
  File "<string>", line 25, in <module>
TypeError: species() got an unexpected keyword argument 'reactive'
Traceback (most recent call last):
  File "/usr/share/miniconda3/envs/rmg_env/lib/python3.7/site-packages/julia/pseudo_python_cli.py", line 308, in main
    python(**vars(ns))
  File "/usr/share/miniconda3/envs/rmg_env/lib/python3.7/site-packages/julia/pseudo_python_cli.py", line 59, in python
    scope = runpy.run_path(script, run_name="__main__")
  File "/usr/share/miniconda3/envs/rmg_env/lib/python3.7/runpy.py", line 263, in run_path
    pkg_name=pkg_name, script_name=fname)
  File "/usr/share/miniconda3/envs/rmg_env/lib/python3.7/runpy.py", line 96, in _run_module_code
    mod_name, mod_spec, pkg_name, script_name)
  File "/usr/share/miniconda3/envs/rmg_env/lib/python3.7/runpy.py", line 85, in _run_code
    exec(code, run_globals)
  File "rmgpy/tools/regression.py", line 203, in <module>
    variables_failed = main()
  File "rmgpy/tools/regression.py", line 197, in main
    args = read_input_file(input_file)  # casetitle, observables, setups, tol
  File "rmgpy/tools/regression.py", line 85, in read_input_file
    exec(f.read(), global_context, local_context)
  File "<string>", line 25, in <module>
TypeError: species() got an unexpected keyword argument 'reactive'
RMS_liquidSurface_ch4o2cat Failed Observable Testing

hwpang marked this pull request as draft May 24, 2023 15:49

hwpang force-pushed the rms_reactor_regression branch 3 times, most recently from 0d607fd to 51e7e59 Compare May 24, 2023 16:23

JacksonBurns mentioned this pull request May 24, 2023

Python 3.7 End-of-Life and Upgrading to Python 3.11 #2445

Closed

JacksonBurns linked an issue May 24, 2023 that may be closed by this pull request

None of our regression tests use RMS reactors #2442

Closed

hwpang force-pushed the rms_reactor_regression branch from 51e7e59 to fad3123 Compare May 24, 2023 17:25

hwpang force-pushed the rms_reactor_regression branch from cbc73de to ada87d8 Compare May 26, 2023 14:58

hwpang force-pushed the rms_reactor_regression branch from ada87d8 to fe52f1e Compare May 26, 2023 20:26

hwpang marked this pull request as ready for review May 30, 2023 15:18

hwpang force-pushed the rms_reactor_regression branch from fe52f1e to d748684 Compare May 30, 2023 15:18

JacksonBurns reviewed May 31, 2023

View reviewed changes

JacksonBurns requested a review from mjohnson541 May 31, 2023 20:01

mjohnson541 reviewed Jun 3, 2023

View reviewed changes

hwpang force-pushed the rms_reactor_regression branch from d748684 to 283c838 Compare June 5, 2023 15:15

hwpang added 3 commits June 7, 2023 14:50

Add new regression tests to CI

0dc6769

Add regression test for RMS's constantVIdealGasReactor using supermin…

f1ec68d

…imal

Add Add regression test for RMS's CSTR using pentane oxidation

df004c2

saveEdgeSpecies should be in options block

hwpang added 3 commits June 7, 2023 14:50

should use tuple, not list. should use position argument, not kwarg

a14149b

Fix bug to use termination conversion in RMS's constant_T_V_liquid_re…

420b3a2

…actor

hwpang force-pushed the rms_reactor_regression branch from 283c838 to 420b3a2 Compare June 7, 2023 18:50

hwpang self-assigned this Jun 7, 2023

hwpang requested review from mjohnson541 and JacksonBurns June 8, 2023 00:15

JacksonBurns approved these changes Jun 8, 2023

View reviewed changes

JacksonBurns added the Python 3.11 Transition PRs and Issues related to transitioning from Python 3.7 to 3.11 label Jun 8, 2023

hwpang merged commit 34d5d87 into ReactionMechanismGenerator:main Jun 8, 2023

hwpang deleted the rms_reactor_regression branch June 8, 2023 12:56

This was referenced Jun 9, 2023

Regression testing updates #2459

Merged

Enable Metal, Facet, site and terrace specification in Molecule #2452

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add regression test for RMS reactor #2446

Add regression test for RMS reactor #2446

hwpang commented May 24, 2023 •

edited

Loading

JacksonBurns commented May 24, 2023

hwpang commented May 24, 2023 •

edited

Loading

hwpang commented May 24, 2023 •

edited

Loading

rwest commented May 24, 2023

hwpang commented May 24, 2023

hwpang commented May 26, 2023

hwpang commented May 26, 2023

mjohnson541 commented May 26, 2023

hwpang commented May 26, 2023

hwpang commented May 30, 2023

JacksonBurns left a comment

mjohnson541 left a comment

hwpang commented Jun 5, 2023 •

edited

Loading

JacksonBurns commented Jun 5, 2023

hwpang commented Jun 7, 2023

hwpang commented Jun 8, 2023 •

edited

Loading

JacksonBurns left a comment

rwest commented Jun 9, 2023

Add regression test for RMS reactor #2446

Add regression test for RMS reactor #2446

Conversation

hwpang commented May 24, 2023 • edited Loading

Motivation or Problem

Description of Changes

JacksonBurns commented May 24, 2023

hwpang commented May 24, 2023 • edited Loading

hwpang commented May 24, 2023 • edited Loading

rwest commented May 24, 2023

hwpang commented May 24, 2023

hwpang commented May 26, 2023

hwpang commented May 26, 2023

mjohnson541 commented May 26, 2023

hwpang commented May 26, 2023

hwpang commented May 30, 2023

JacksonBurns left a comment

Choose a reason for hiding this comment

mjohnson541 left a comment

Choose a reason for hiding this comment

hwpang commented Jun 5, 2023 • edited Loading

JacksonBurns commented Jun 5, 2023

hwpang commented Jun 7, 2023

hwpang commented Jun 8, 2023 • edited Loading

JacksonBurns left a comment

Choose a reason for hiding this comment

rwest commented Jun 9, 2023

hwpang commented May 24, 2023 •

edited

Loading

hwpang commented May 24, 2023 •

edited

Loading

hwpang commented May 24, 2023 •

edited

Loading

hwpang commented Jun 5, 2023 •

edited

Loading

hwpang commented Jun 8, 2023 •

edited

Loading