-
Notifications
You must be signed in to change notification settings - Fork 245
Added completedNetworks option to prevent exploring PDep networks that are complete. #2853
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
372e086
to
39d6510
Compare
Regression Testing Results
Detailed regression test results.Regression test aromatics:Reference: Execution time (DD:HH:MM:SS): 00:00:00:50 aromatics Passed Core Comparison ✅Original model has 15 species. aromatics Failed Edge Comparison ❌Original model has 106 species. Non-identical thermo! ❌
Identical thermo comments:
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:01 liquid_oxidation Passed Core Comparison ✅Original model has 37 species. liquid_oxidation Failed Edge Comparison ❌Original model has 214 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:03 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(oxirene) + 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:01: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 ✅Errors occurred during observable testing
WARNING:root:Initial mole fractions do not sum to one; normalizing.
|
Probably won't speed it up much, but we might as well.
…t are complete. Sometimes you have a pressure-dependent network that has been studied in great detail and put in a seed mechanism or reaction library, (such as CH2O2) and you don't want RMG to add reactions to the network and your model, because doing so will make it worse. Any reactions net reactions that are in your seed mechanism won't be changed, but currently if there's a new reaction RMG would add it. This option lets you specify in the input file that you don't want RMG to expand certain networks further. You specify them with a line in your pressureDependence block of your input file like completedNetworks = ['CH2O2', 'C2H6'],
39d6510
to
242798e
Compare
Motivation or Problem
Sometimes you have a pressure-dependent network that has been studied in great detail and put in a seed mechanism or reaction library, (such as CH2O2) and you don't want RMG to add reactions to the network and your model, because doing so will make it worse.
Any net reactions that are in your seed mechanism won't be changed by RMG's estimates, but currently if RMG finds a new reaction then it will add it. If the original authors did a thorough job and still left out a reaction, it's because they think it is slow, and adding RMG's estimate will probably make your model worse.
@Nora-Khalil has been running into this problem (it's not just hypothetical), although I'm not sure if we have an issue open for it yet.
Description of Changes
This PR adds an option that lets you specify in the input file that you don't want RMG to expand certain networks further.
You specify them with a line in your pressureDependence block of your input file like
completedNetworks = ['CH2O2'],
This can be a list of things, specified by chemical formula of the potential energy surface that has been explored.
Testing
I added a line to an example input file, for testing, but this is poor example (preventing ethane from reacting in the ethane oxidation example!) and we should change it before shipping this, so we're not distributing bad examples.
But for now, it demonstrates the new feature.
Reviewer Tips
See if it works as intended, in a better example than the one I gave. Something where the PDep network is complete in the seed mechanism.
Up for debate
I wasn't sure where in the input file this should go.
The list of networks is stored in the
rmg.reaction_model
object, not thermg.pressure_dependence
, but I think it only makes sense when you have pressure dependence on, so I put it in the pressureDependence block of the input file. I also considered thegeneratedSpeciesConstraints
block. Or another block. But maybe we're rewriting all the input files in yaml soon anyway...We also considered in off-line discussion whether this should in fact live in the reaction library or seed mechanism, and you get networks marked as "complete" according to the seed mechanism you use. But that would involve editing the database also, and so for this testing of the concept I didn't try that. But would that be better in the long run?
Still to do (WIP)