Fix chemkin file duplicate flags #1856
Conversation
rwest
force-pushed
the
chemkin_duplicates
branch
from
e11bfdd
to
d0cd0ff
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1856 +/- ##
=======================================
Coverage 55.32% 55.32%
=======================================
Files 125 125
Lines 37139 37139
=======================================
Hits 20547 20547
Misses 16592 16592 ☔ View full report in Codecov by Sentry. |
|
Just built a model using these commits, which resulted in a declared duplicate not being found: I think this is because the reaction |
|
The problem with things being declared as duplicates that aren't, was indeed an error in the NOx2018 library. This is addressed in ReactionMechanismGenerator/RMG-database#377 but we should probably still add checks for this type of error to the automated database tests. |
|
Note that although I think this fixes a problem that we had in identifying the duplicate reactions and labeling them as such for valid chemkin files, it does not address the question as to why we were making two such reactions in the first place. That might require more investigation. |
rwest
force-pushed
the
chemkin_duplicates
branch
from
d0cd0ff
to
a535889
Compare
|
When using ck2cti, I get this error of unmarked duplicate reactions but have only found error warning on reactions of the following format: These two reactions will be flagged as unmarked duplicates, and will have to be manually changed before the ck2cti command runs successfully. |
|
How does it behave in the latest ck2yaml? Same? |
|
Haven't tried that, but can try it now |
|
Was trying to use ck2cti on a chemkin file for a halocarbon (CH2FCHF2). This mechanism has the following two reactions included in it: `S(879)+S(879)(+M)=S(878)+S(878)(+M) 1.000e+00 0.000 0.000 S(878)(+M)=S(879)(+M) 1.000e+00 0.000 0.000 Note that RMG had not marked these as duplicate reactions. I had previously been converting the files while on David Farina's halocarbon combustion branch. When running the ck2cti command, I got multiple errors like this one:
This error almost always pertains to pressure dependent reactions in the form A+B(+M)=B+C(+M) or A(+M)=B+C(+M), with the two unmarked reactions having the same exact equation but different rates. A year ago, I created a script that handles these errors, adding in DUPLICATES where needed in the file. But lately, I've been running into an error with unmarked duplicates in the form A(+M)=B(+M) & A+A=B+B, with the error message as (for this example):
When running the ck2yaml command, NO errors like |
| same_dir_match = (reaction1.reactants == reaction2.reactants and reaction1.products == reaction2.products) | ||
| opposite_dir_match = (reaction1.products == reaction2.reactants and reaction1.reactants == reaction2.products) | ||
| same_dir_match = (sorted(reaction1.reactants) == sorted(reaction2.reactants) and sorted(reaction1.products) == sorted(reaction2.products)) | ||
| opposite_dir_match = (sorted(reaction1.products) == sorted(reaction2.reactants) and sorted(reaction1.reactants) == sorted(reaction2.products)) |
There was a problem hiding this comment.
There may need to be an edit here to catch errors like
InputFileError thrown by Kinetics::checkDuplicates: Undeclared duplicate reactions detected: Reaction ####: A <=> A Reaction ####: 2 A <=> 2 B
that results when you use either the ck2cti or ck2yaml.
|
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. |
rwest
force-pushed
the
chemkin_duplicates
branch
from
a535889
to
bf4a8a4
Compare
Regression Testing Results==> test/regression-diff/liquid_oxidation-edge.log <== Non-identical kinetics! ❌
kinetics: ==> test/regression-diff/nitrogen-core.log <== ==> test/regression-diff/nitrogen-edge.log <== 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: ==> test/regression-diff/sulfur-edge.log <== beep boop this action was performed by a bot 🤖 |
rwest
force-pushed
the
chemkin_duplicates
branch
2 times, most recently
from
2f1b0fa
to
13be032
Compare
Regression Testing ResultsTraceback (most recent call last): During handling of the above exception, another exception occurred: Traceback (most recent call last): Detailed regression test results.Regression test aromatics:Reference: Execution time (DD:HH:MM:SS): 00:00:01:23 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)(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 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:
Observables 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:02:47 liquid_oxidation Failed Core Comparison ❌Original model has 37 species. Non-identical kinetics! ❌
kinetics: liquid_oxidation Failed Edge Comparison ❌Original model has 202 species. Non-identical kinetics! ❌
kinetics: Non-identical kinetics! ❌
kinetics:
Observables 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:01:44 nitrogen Failed Core Comparison ❌Original model has 41 species. nitrogen Failed Edge Comparison ❌Original model has 132 species.
Observables 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:02:50 oxidation Passed Core Comparison ✅Original model has 59 species. oxidation Passed Edge Comparison ✅Original model has 230 species.
Observables 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 ✅Regression test sulfur:Reference: Execution time (DD:HH:MM:SS): 00:00:01:08 sulfur Passed Core Comparison ✅Original model has 27 species. sulfur Failed Edge Comparison ❌
Observables Test Case: SO2 Comparison
✅ All Observables varied by less than 0.100 on average between old model and new model in all conditions! sulfur Passed Observable Testing ✅Regression test superminimal:Reference: Execution time (DD:HH:MM:SS): 00:00:00:43 superminimal Passed Core Comparison ✅Original model has 13 species. superminimal Passed Edge Comparison ✅Original model has 18 species. Regression test RMS_constantVIdealGasReactor_superminimal:Reference: Execution time (DD:HH:MM:SS): 00:00:03:21 RMS_constantVIdealGasReactor_superminimal Passed Core Comparison ✅Original model has 13 species. RMS_constantVIdealGasReactor_superminimal Passed Edge Comparison ✅Original model has 13 species.
Observables Test Case: RMS_constantVIdealGasReactor_superminimal Comparison
✅ All Observables varied by less than 0.100 on average between old model and new model in all conditions! RMS_constantVIdealGasReactor_superminimal Passed Observable Testing ✅Regression test RMS_CSTR_liquid_oxidation:Reference: Execution time (DD:HH:MM:SS): 00:00:08:16 RMS_CSTR_liquid_oxidation Failed Core Comparison ❌Original model has 37 species. RMS_CSTR_liquid_oxidation Failed Edge Comparison ❌Original model has 206 species. Non-identical kinetics! ❌
kinetics:
Observables Test Case: RMS_CSTR_liquid_oxidation Comparison
✅ All Observables varied by less than 0.100 on average between old model and new model in all conditions! RMS_CSTR_liquid_oxidation Passed Observable Testing ✅beep boop this comment was written by a bot 🤖 |
rwest
force-pushed
the
chemkin_duplicates
branch
from
13be032
to
a1ca9c7
Compare
Regression Testing ResultsTraceback (most recent call last): During handling of the above exception, another exception occurred: Traceback (most recent call last): Detailed regression test results.Regression test aromatics:Reference: Execution time (DD:HH:MM:SS): 00:00:01:24 aromatics Passed Core Comparison ✅Original model has 15 species. aromatics Passed Edge Comparison ✅Original model has 106 species.
Observables 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:02:51 liquid_oxidation Failed Core Comparison ❌Original model has 37 species. Non-identical kinetics! ❌
kinetics: liquid_oxidation Failed Edge Comparison ❌Original model has 202 species. Non-identical kinetics! ❌
kinetics:
Observables 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:01:45 nitrogen Failed Core Comparison ❌Original model has 41 species. nitrogen Failed Edge Comparison ❌Original model has 132 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:
Observables 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:02:51 oxidation Passed Core Comparison ✅Original model has 59 species. oxidation Passed Edge Comparison ✅Original model has 230 species.
Observables 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 ✅Regression test sulfur:Reference: Execution time (DD:HH:MM:SS): 00:00:01:10 sulfur Passed Core Comparison ✅Original model has 27 species. sulfur Failed Edge Comparison ❌
Observables Test Case: SO2 Comparison
✅ All Observables varied by less than 0.100 on average between old model and new model in all conditions! sulfur Passed Observable Testing ✅Regression test superminimal:Reference: Execution time (DD:HH:MM:SS): 00:00:00:44 superminimal Passed Core Comparison ✅Original model has 13 species. superminimal Passed Edge Comparison ✅Original model has 18 species. Regression test RMS_constantVIdealGasReactor_superminimal:Reference: Execution time (DD:HH:MM:SS): 00:00:03:28 RMS_constantVIdealGasReactor_superminimal Passed Core Comparison ✅Original model has 13 species. RMS_constantVIdealGasReactor_superminimal Passed Edge Comparison ✅Original model has 13 species.
Observables Test Case: RMS_constantVIdealGasReactor_superminimal Comparison
✅ All Observables varied by less than 0.100 on average between old model and new model in all conditions! RMS_constantVIdealGasReactor_superminimal Passed Observable Testing ✅Regression test RMS_CSTR_liquid_oxidation:Reference: Execution time (DD:HH:MM:SS): 00:00:08:16 RMS_CSTR_liquid_oxidation Failed Core Comparison ❌Original model has 37 species. RMS_CSTR_liquid_oxidation Failed Edge Comparison ❌Original model has 206 species.
Observables Test Case: RMS_CSTR_liquid_oxidation Comparison
The following observables did not match: ❌ Observable species CCCCC varied by more than 0.100 on average between old model pentane(2) and new model pentane(2) in condition 1.
RMS_CSTR_liquid_oxidation Failed Observable Testing ❌beep boop this comment was written by a bot 🤖 |
There's a model from Klippenstein/Glarborg, with three duplicate reactions, two PDep and one not. The previous implementation would notice that because one is PDep and one is not, it would REMOVE the DUPLICATE flag, even though there's a THIRD reaction that means the DUPLICATE flag should remain. Now we just log a warning and leave the flag alone.
The new code doesn't remove duplicate reaction marks, so this test is marked as a work in progress. Unless we decide the current behavior is ideal, in which case we can change the expected outcomes and make this pass.
rwest
force-pushed
the
chemkin_duplicates
branch
from
a1ca9c7
to
186a20e
Compare
Regression Testing ResultsTraceback (most recent call last): During handling of the above exception, another exception occurred: Traceback (most recent call last): Detailed regression test results.Regression test aromatics:Reference: Execution time (DD:HH:MM:SS): 00:00:01:17 aromatics Passed Core Comparison ✅Original model has 15 species. aromatics Passed Edge Comparison ✅Original model has 106 species.
Observables 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:02:39 liquid_oxidation Failed Core Comparison ❌Original model has 37 species. liquid_oxidation Failed Edge Comparison ❌Original model has 202 species. Non-identical kinetics! ❌
kinetics:
Observables 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:01:38 nitrogen Failed Core Comparison ❌Original model has 41 species. nitrogen Failed Edge Comparison ❌Original model has 132 species.
Observables 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:02:40 oxidation Passed Core Comparison ✅Original model has 59 species. oxidation Passed Edge Comparison ✅Original model has 230 species.
Observables 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 ✅Regression test sulfur:Reference: Execution time (DD:HH:MM:SS): 00:00:01:04 sulfur Passed Core Comparison ✅Original model has 27 species. sulfur Failed Edge Comparison ❌
Observables Test Case: SO2 Comparison
✅ All Observables varied by less than 0.100 on average between old model and new model in all conditions! sulfur Passed Observable Testing ✅Regression test superminimal:Reference: Execution time (DD:HH:MM:SS): 00:00:00:40 superminimal Passed Core Comparison ✅Original model has 13 species. superminimal Passed Edge Comparison ✅Original model has 18 species. Regression test RMS_constantVIdealGasReactor_superminimal:Reference: Execution time (DD:HH:MM:SS): 00:00:03:05 RMS_constantVIdealGasReactor_superminimal Passed Core Comparison ✅Original model has 13 species. RMS_constantVIdealGasReactor_superminimal Passed Edge Comparison ✅Original model has 13 species.
Observables Test Case: RMS_constantVIdealGasReactor_superminimal Comparison
✅ All Observables varied by less than 0.100 on average between old model and new model in all conditions! RMS_constantVIdealGasReactor_superminimal Passed Observable Testing ✅Regression test RMS_CSTR_liquid_oxidation:Reference: Execution time (DD:HH:MM:SS): 00:00:07:38 RMS_CSTR_liquid_oxidation Failed Core Comparison ❌Original model has 37 species. RMS_CSTR_liquid_oxidation Failed Edge Comparison ❌Original model has 206 species.
Observables Test Case: RMS_CSTR_liquid_oxidation Comparison
✅ All Observables varied by less than 0.100 on average between old model and new model in all conditions! RMS_CSTR_liquid_oxidation Passed Observable Testing ✅beep boop this comment was written by a bot 🤖 |
This is a problem with either |
|
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. |
|
Worth keeping alive and merging?? |
|
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. |
github-actions
bot
added
stale
Motivation or Problem 1
#1823 and #1336 describe problems with Chemkin files having unmarked duplicate reactions, which then causes problems in Cantera.
The very first example in #1336 I can't explain, but nobody can reproduce that behavior anyway.
All the other examples have the property that either the reactants or the products are in a different order., eg. one reaction is
A + B = C + Dand the undetected duplicate is eitherA + B = D + CorB + A = C + D.The function
chemkin.mark_duplicate_reactionwas not sorting the reactants or products before comparing the lists.Description of Changes 1
The main change is sorting the reactants and products before comparing them.
This should address #1823 and #1336.
But there's another duplicate flag problem addressed here too.
Motivation or Problem 2
The reaction library
NOx2018has the reactionHCCO + OH <=> HCOH + COdescribed as three duplicate reactions:I think this in accurate representation of the mess in the published Chemkin file:
But the algorithm to detect and remove un-necessary DUPLICATE markers is over-zealous.
Because non-pdep and pdep reactions usually do not need to be marked as duplicates, it removes the duplicate markers. eg.
However, there are two pdep reactions, and so the marker should remain.
Description of Changes 2
It now just logs a warning that the DUPLICATE flag may be redundant, but leaves it in place. Hopefully these are only marked as duplicates appropriately anyway.
Testing
I ran the existing unit tests. I'm currently re-building a large model that revealed these errors. I will know soon whether they went away.