Closed hwpang closed 1 year ago
It looks like there's an issue with one of the reaction simulation tests comparing to past simulation results.
The test failed because the kH in the old mechanism files aren't using the definition of kH_i = Pgas_i/Cliq_i. I use RMG to re-estimate the parameters in the old mechanism file and update them.
Merging #204 (27fde69) into main (1ccd7fe) will not change coverage. The diff coverage is
100.00%
.
@@ Coverage Diff @@
## main #204 +/- ##
=======================================
Coverage 48.73% 48.73%
=======================================
Files 31 31
Lines 7567 7567
=======================================
Hits 3688 3688
Misses 3879 3879
Impacted Files | Coverage Δ | |
---|---|---|
src/TestReactors.jl | 100.00% <ø> (ø) |
|
src/Interface.jl | 88.71% <100.00%> (ø) |
:mega: We’re building smart automated test selection to slash your CI/CD build times. Learn more
Previously I wrote the equation to use the definition of
kH_i = Cgas_i/Cliq_i
. There are many definitions of kH out there, usually a combination of Cgas_i, xgas_i, Pgas_i, Cliq_i, and xliq_i. I had a PR to change the definition of kH estimated by RMG into Pgas_i/Cliq_i (https://github.com/ReactionMechanismGenerator/RMG-Py/pull/2389). This definition is better in terms of Jacobian sparsity than the definition of Pgas_i/xliq_i, because the latter requires Cliqtot = sum(Cliq) during the computation of fluxes. I believe that this new definition is much more beneficial in the sense of simulation.