Conversation
during model initialization
which runs with the regression tests
There was a problem hiding this comment.
Pull request overview
This PR addresses an initialization-time crash in RMG model generation when a non-reactive (inert) initial species (e.g., N2) appears in reactions loaded from kinetics libraries, by ensuring thermo is generated for all initial species before reaction libraries are added to the edge.
Changes:
- Generate thermo for all
initial_speciesduring model initialization (not onlyspec.reactivespecies). - Update an example RMG input to include the
NOx2018reaction library to reproduce the reported scenario.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
rmgpy/rmg/main.py |
Ensures thermo is submitted for non-reactive initial species prior to loading reaction libraries, preventing missing-thermo failures during library reaction processing. |
examples/rmg/1,3-hexadiene/input.py |
Adds NOx2018 reaction library to the example input to exercise the inert-N2 + library workflow. |
| thermoLibraries = ['primaryThermoLibrary', 'GRI-Mech3.0'], | ||
| reactionLibraries = [], | ||
| reactionLibraries = ['NOx2018'], | ||
| seedMechanisms = [], |
There was a problem hiding this comment.
The PR description says the change is covered by running a modified input in regression tests, but this PR only updates an examples/ input file. If CI doesn’t execute examples, the crash scenario (non-reactive species participating in reaction-library reactions during initialization) won’t be exercised automatically. Consider adding/adjusting a regression/functional test input under test/regression/ (e.g., a nitrogen case that includes an inert N2 plus a library containing N2-involving reactions) so this behavior is validated in CI.
| submit(spec, self.solvent) | ||
| if vapor_liquid_mass_transfer.enabled: | ||
| spec.get_liquid_volumetric_mass_transfer_coefficient_data() | ||
| spec.get_henry_law_constant_data() |
There was a problem hiding this comment.
submit(spec, self.solvent) is now called for non-reactive initial species here, but the same self.initial_species loop later in initialize() calls submit() again (around the subsequent loop before core/edge enlargement). Since thermoengine.submit() recomputes thermo unconditionally, this change makes non-reactive species pay that cost twice as well. Consider refactoring so each initial species is submitted only once (or guard the later loop to skip species with thermo already computed) while still ensuring thermo is available before loading reaction libraries.
| submit(spec, self.solvent) | |
| if vapor_liquid_mass_transfer.enabled: | |
| spec.get_liquid_volumetric_mass_transfer_coefficient_data() | |
| spec.get_henry_law_constant_data() | |
| # Compute thermo and any solvent-dependent properties early only for | |
| # reactive species; non-reactive species will still have thermo | |
| # handled later, avoiding unnecessary duplicate work here. | |
| if spec.reactive: | |
| submit(spec, self.solvent) | |
| if vapor_liquid_mass_transfer.enabled: | |
| spec.get_liquid_volumetric_mass_transfer_coefficient_data() | |
| spec.get_henry_law_constant_data() |
Regression Testing Results
Detailed regression test results.Regression test aromatics:Reference: Execution time (DD:HH:MM:SS): 00:00:00:53 aromatics Passed Core Comparison ✅Original model has 15 species. aromatics Failed Edge Comparison ❌Original model has 106 species. Non-identical thermo! ❌
thermo: Thermo group additivity estimation: group(Cs-(Cds-Cds)(Cds-Cds)CsCs) + group(Cs-(Cds-Cds)CsCsH) + group(Cs-(Cds-Cds)CsCsH) + group(Cs-CsCsHH) + group(Cds-CdsCsH) + group(Cds-CdsCsH) + group(Cds-CdsCsH) + group(Cds-CdsCsH) + polycyclic(s2_4_4_ene_1) + polycyclic(s1_4_5_diene_1_6) + polycyclic(s3_4_5_ene_1) - ring(Cyclobutene) - ring(Cyclobutane) - ring(Cyclopentene) + radical(bicyclo[2.1.1]hex-2-ene-C5) Non-identical thermo! ❌
thermo: Thermo group additivity estimation: group(Cs-(Cds-Cds)(Cds-Cds)(Cds-Cds)H) + group(Cds-Cds(Cds-Cds)(Cds-Cds)) + group(Cds-CdsCsH) + group(Cds-CdsCsH) + group(Cds-Cds(Cds-Cds)H) + group(Cds-Cds(Cds-Cds)H) + group(Cds-CdsCsH) + group(Cdd-CdsCds) + Estimated bicyclic component: polycyclic(s4_6_6_ane) - ring(Cyclohexane) - ring(Cyclohexane) + ring(124cyclohexatriene) + ring(124cyclohexatriene) Non-identical thermo! ❌
thermo: Thermo group additivity estimation: group(Cs-(Cds-Cds)(Cds-Cds)CsH) + group(Cs-(Cds-Cds)CsHH) + group(Cds-Cds(Cds-Cds)(Cds-Cds)) + group(Cds-CdsCsH) + group(Cds-CdsCsH) + group(Cds-CdsCsH) + group(Cds-Cds(Cds-Cds)H) + group(Cds-Cds(Cds-Cds)H) + Estimated bicyclic component: polycyclic(s4_6_6_ane) - ring(Cyclohexane) - ring(Cyclohexane) + ring(1,3-Cyclohexadiene) + ring(1,4-Cyclohexadiene) Non-identical thermo! ❌
thermo: Thermo group additivity estimation: group(Cs-(Cds-Cds)(Cds-Cds)CsH) + group(Cs-(Cds-Cds)(Cds-Cds)CsH) + group(Cds-Cds(Cds-Cds)(Cds-Cds)) + group(Cds- CdsCsH) + group(Cds-CdsCsH) + group(Cds-CdsCsH) + group(Cds-CdsCsH) + group(Cds-Cds(Cds-Cds)H) + group(Cds-Cds(Cds-Cds)H) + group(Cds-CdsHH) + Estimated bicyclic component: polycyclic(s4_6_6_ane) - ring(Cyclohexane) - ring(Cyclohexane) + ring(1,3-Cyclohexadiene) + ring(1,4-Cyclohexadiene) + radical(Cds_P) Non-identical thermo! ❌
thermo: Thermo group additivity estimation: group(Cs-(Cds-Cds)(Cds-Cds)CsH) + group(Cs-(Cds-Cds)(Cds-Cds)CsH) + group(Cds-Cds(Cds-Cds)(Cds-Cds)) + group(Cds- CdsCsH) + group(Cds-CdsCsH) + group(Cds-CdsCsH) + group(Cds-CdsCsH) + group(Cds-Cds(Cds-Cds)H) + group(Cds-Cds(Cds-Cds)H) + group(Cds-CdsHH) + Estimated bicyclic component: polycyclic(s4_6_6_ane) - ring(Cyclohexane) - ring(Cyclohexane) + ring(1,3-Cyclohexadiene) + ring(1,4-Cyclohexadiene) Non-identical kinetics! ❌
kinetics: Non-identical kinetics! ❌
kinetics: Non-identical kinetics! ❌
kinetics: Non-identical kinetics! ❌
kinetics: Non-identical kinetics! ❌
kinetics: Non-identical kinetics! ❌
kinetics: Non-identical kinetics! ❌
kinetics: Non-identical kinetics! ❌
kinetics: Non-identical kinetics! ❌
kinetics: Non-identical kinetics! ❌
kinetics: Non-identical kinetics! ❌
kinetics: Non-identical kinetics! ❌
kinetics: Non-identical kinetics! ❌
kinetics: Non-identical kinetics! ❌
kinetics: Non-identical kinetics! ❌
kinetics: Non-identical kinetics! ❌
kinetics: Non-identical kinetics! ❌
kinetics: Non-identical kinetics! ❌
kinetics: Non-identical kinetics! ❌
kinetics: Non-identical kinetics! ❌
kinetics: DetailsObservables Test Case: Aromatics Comparison✅ All Observables varied by less than 0.500 on average between old model and new model in all conditions! aromatics Passed Observable Testing ✅Regression test liquid_oxidation:Reference: Execution time (DD:HH:MM:SS): 00:00:01:53 liquid_oxidation Passed Core Comparison ✅Original model has 37 species. liquid_oxidation Failed Edge Comparison ❌Original model has 214 species. DetailsObservables Test Case: liquid_oxidation Comparison✅ All Observables varied by less than 0.100 on average between old model and new model in all conditions! liquid_oxidation Passed Observable Testing ✅Regression test nitrogen:Reference: Execution time (DD:HH:MM:SS): 00:00:00:59 nitrogen Passed Core Comparison ✅Original model has 41 species. nitrogen Failed Edge Comparison ❌Original model has 133 species. Non-identical thermo! ❌
thermo: Thermo group additivity estimation: group(O2s-CdN3d) + group(N3d-OCd) + group(Cd-HN3dO) + ring(Cyclopropene) + radical(CdJ-NdO) Non-identical kinetics! ❌
kinetics: DetailsObservables Test Case: NC Comparison✅ All Observables varied by less than 0.200 on average between old model and new model in all conditions! nitrogen Passed Observable Testing ✅Regression test oxidation:Reference: Execution time (DD:HH:MM:SS): 00:00:01:39 oxidation Passed Core Comparison ✅Original model has 59 species. oxidation Passed Edge Comparison ✅Original model has 230 species. DetailsObservables Test Case: Oxidation Comparison✅ All Observables varied by less than 0.500 on average between old model and new model in all conditions! oxidation Passed Observable Testing ✅Errors occurred during observable testing
WARNING:root:Initial mole fractions do not sum to one; normalizing.
|
Motivation or Problem
When generating a (PDep) model with N2 as an inert species and including kinetic libraries such as NOx2018, RMG loads to the edge reactions such
NNH <=> N2 + Hand crashes with:Description of Changes
We could either tell RMG not to load reactions with inert species, or load thermo for inert species as well. In my view, both are harmless just the same. Here, we tell RMG to always load thermo, even for inert species.
Testing
Modified an existing RMG example input file with inert N2 to include the NOx2018 library, it should run in the regression tests.