-
Notifications
You must be signed in to change notification settings - Fork 244
Fix E0 energy_correction in pressure dependence calculations. #2835
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
Regression Testing Results
Detailed regression test results.Regression test aromatics:Reference: Execution time (DD:HH:MM:SS): 00:00:00:49 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:01:55 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:00:58 nitrogen Passed Core Comparison ✅Original model has 41 species. nitrogen Passed Edge Comparison ✅Original model has 133 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:01:30 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.
|
The property E0 when accessed from a Configuration, includes the energy_correction which was added in 169a34e
This could help when reporting Configuration objects, and at least makes debugging less misleading. Without this offset, all your energies are wrong. See #2791. But this alone doesn't fix the issue since Configuration objects are not printed inte Arkane input files using __repr__. We should generally have better unit tests that __repr__ reproduces things.
As noted in #2791 the energy_correction was not actually an average of the Configuration energies because some Configurations (reactants and products) are bimolecular. This corrects the calculation, but also switches to using the MINIMUM rather than the average, so that the lowest should end up at zero.
…file The E0 values for TS have been modified using the energy_correction, but the E0 values for species have not been. We can't change the species, because it needs to be applied to the Configuration (some are bimolecular, some are unimolecular), so to be consistent we must remove it from the TS. This leaves both values, and a comment. It's a bit ugly, and I think there should be a better fix later, but this I think solves an immediate bug, and now the network_X_Y.py files written during an RMG job are at least internally consistent. (They have a different definition of 0 from inside the RMG job, but they're self-consistent.)
When comparing mechanisms that are NOT surface models, it would do the load_chemkin_file as part of an `except` block, which then means if you get an error during `load_chemkin_file` the stack trace and error message are all confused and it looks like a KeyError: 'surface_path1' which is misleading.
For the Chebyshev fitting it used to ignore the specified limits and just use the lowest and highest points from the grid. Chebyshev grid points aren't on the boundaries of the valid range, so you'd end up with not the range you specified. One consequence is network.py files created during an RMG job would not reproduce the fits if you later ran them in Arkane. Without this change, an RMG job would create TCHEB/ 300.000 2500.000 / PCHEB/ 0.010 98.692 / but then when you run the network1_1.py file through Arkane you would get TCHEB/ 302.558 2335.460 / PCHEB/ 0.012 78.776 /
the {reaction:s} was causing a TypeError: unsupported format string passed to TemplateReaction.__format__ but {reaction} should do the intended conversion. This line is seldom hit.
Because the T limits were being read incorrectly, a test of the rate coefficient that was supposedly at Tmax (but was in fact at 700K) was failing once the T limit reading was fixed. Also the pressure limit reading test was using .value instead of .value_si so that broke also.
This was harder to read, parse, understand, or maintain. Also added a few extra asserts (units etc.)
Regression Testing Results
Detailed regression test results.Regression test aromatics:Reference: Execution time (DD:HH:MM:SS): 00:00:00:49 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(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:
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:01:57 liquid_oxidation Passed Core Comparison ✅Original model has 37 species. liquid_oxidation Failed Edge Comparison ❌Original model has 214 species.
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:02 nitrogen Passed Core Comparison ✅Original model has 41 species. nitrogen Passed Edge Comparison ✅Original model has 133 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:01:48 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.
|
Motivation or Problem
In #2791 @donerancl revealed some problems with the energy_correction that is applied to PDep networks. The energy_correction was not being calculated correctly for bimolecular configurations, and the E0 values written to Arkane input files were inconsistent between transition states and species. Additionally, temperature and pressure limits specified in Arkane jobs were being ignored in favor of grid boundaries, causing reproducibility issues between RMG runs and subsequent Arkane runs.
Description of Changes
PressureDependenceJob
to respect user-specified Tmin/Tmax/Pmin/Pmax instead of always using grid boundaries. Fixed unit tests that were failing due to the corrected T limit reading.Other fixes made along the way
Testing
arkane/commonTest.py
that were inadvertently testing at wrong temperatures