NH3 thermo and kinetic data additions #620
Conversation
alongd
force-pushed
the
nh3
branch
3 times, most recently
from
7c20f0f to
58fda23
Compare
alongd
force-pushed
the
nh3
branch
5 times, most recently
from
a4c81cc to
8a58f39
Compare
alongd
force-pushed
the
nh3
branch
3 times, most recently
from
69c4794 to
7b90618
Compare
alongd
force-pushed
the
nh3
branch
2 times, most recently
from
06303e8 to
64b72d4
Compare
alongd
changed the title
There was a problem hiding this comment.
Hi Alon, thank you and your team for this big update of the thermo database. It can be difficult and tedious work to search carefully through the literature - so once again, thank you for your effort!
Due to the large size of this PR, I will split my review into multiple stages. This will help me get feedback to you as soon as possible rather than all at once.
I double-checked the new data added in primaryNitrogenLibrary, primaryNS, primaryH2O2 and HydrazinePDep. I started looking at forbiddenStructures but will continune in a future review. I haven't yet looked at NH3 library. Here are my overall comments:
-
We have some concerns about forbidding these structures; a species may exist even if it cannot be found at a certain QM LOT. If we forbid a structure that ends up being an important intermediate, this could dramatically - and silently - impair model performance. On Bill's suggestion, I've started commenting on those ones that may need additional discussion, as several pathways in principle seem reasonable for generating those species (and I will examine the rest of the reactions in a future review). Could you please comment about your thoughts on this issue?
-
Many reactions had their kinetic data replaced with newer data, could you briefly comment on this - were such old data in serious error?
-
Some of the data seem to be transcribed in error, please check out the individual line comments.
This comment was marked as off-topic.
This comment was marked as off-topic.
|
Thanks for checking the review comments. There are still a few small loose ends in the new library data, but those are just a matter of checking the original sources. I checked the The main question right now is whether we should merge the forbidden structures. I see the merit in excluding those potentially problematic species, but I also worry that some of them are real intermediates that could play a key role. One option is to not forbid those structures, and then in our subsequent modeling work, see if they form in any appreciable amounts, and perhaps do a deeper analysis at that point. Is that what you suggested? Either is okay with me, but since this would affect the modeling workflow for the ammonia project, we should discuss with the team. |
|
@jonwzheng, I addressed the last comment on the PNL. Please let me know whether there are additional comments or whether I missed anything. I truly appreciate the time and effort of double-checking the values in this library, I know this is a relatively large PR that requires some time and effort to review. I removed most of the forbidden species, let only two that were studied seriously. I squashed the modifications, I think the PR is ready to be merged, of course let me know if there are any additional comments or concerns. |
| ) | ||
|
|
||
| entry( | ||
| index=419, |
There was a problem hiding this comment.
My last comment is that the indexing changes since 417 and 418 were removed. I don't know whether this will make a practical difference, but best to make the indexing consistent anyway.
Regression Testing ResultsWARNING:root:Initial mole fractions do not sum to one; normalizing. Detailed regression test results.Regression test aromatics:Reference: Execution time (DD:HH:MM:SS): 00:00:01:09 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:15 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:25 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:31 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:00:56 sulfur Passed Core Comparison ✅Original model has 27 species. sulfur Failed Edge Comparison ❌Original model has 89 species.
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:41 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:02:26 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:06:01 RMS_CSTR_liquid_oxidation Passed 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 🤖 |
| shortDesc = u"""[Dievart2020]""", | ||
| index=432, | ||
| label="N2H3 + NH2 <=> H2NN(S) + NH3", | ||
| duplicate=True, |
There was a problem hiding this comment.
There are two reactions (this reaction and the last reaction) with index=432. Please move this reaction to the end and change the index number.
|
@ChgCao, I updated the indices and squashed, if the tests pass I think we should be good to go |
Regression Testing ResultsWARNING:root:Initial mole fractions do not sum to one; normalizing. Detailed regression test results.Regression test aromatics:Reference: Execution time (DD:HH:MM:SS): 00:00:01:08 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:08 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:23 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:29 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:00:56 sulfur Passed Core Comparison ✅Original model has 27 species. sulfur Failed Edge Comparison ❌Original model has 89 species.
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:39 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:02:20 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:05:51 RMS_CSTR_liquid_oxidation Passed 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 🤖 |
|
LGTM! but given the large scope @ChgCao could you also take a final look? If you approve, I'll go ahead and merge it in. |
|
Chuangchuang approves (in Slack)! |
Added the
NH3therno library based on CCSD(T)-F12/cc-pVTZ-F12//B2PLYPD3/aug-cc-pVTZ computations (enthalpies taken from ATcT where available)Made significant modifications and additions to the kinetic
promaryNitrogenLibrarybased on updated literature and based on the submitted "NH3-1" paperAlso added several forbidden species and made two minor modifications to the
primaryNitrogenLibrarykinetic libraryWe should still add PDep reactions calculated by us on the N2H3 surface, and later we expect to have updated reactions of the N2H4 surface.
Points for discussion:
NH2 + OH <=> NH(S} + H2O. One updated source that studies allNH2/NH3 + OHreaction at all multiplicities is this, though it is very confusing, and some of the rates are still very high. This is currently unresolved.