Conversation
instead of the old from CTI
|
@JacksonBurns suggested in #2870 (comment) that we might want to force Cantera >= 3.2 rather than Cantera >= 3.0. I think there's a commit in this branch that works around the specific bug he points out (about the ck2yaml in some versions of Cantera requiring that surfaces have a name) so we don't need to avoid it for that reason. But I'm not against using newer things. Strangely, the CI for this run all seemed to choose 3.1.0: Python 3.9 macos-15-intel cantera 3.1.0 py39h921bf41_1 conda-forge What do you think? Force everyone to use 3.2+ ? |
|
conda-forge does not have Cantera 3.2.0 binaries for Python 3.9. |
˙◠˙ Well, we have something else to look forward to when we finish #2796 |
|
And I'm not sure if I ever got a workaround actually in place - this was (I think) my last attempt: ea0b252 |
Regression Testing Results
Detailed regression test results.Regression test aromatics:Reference: Execution time (DD:HH:MM:SS): 00:00:00:56 aromatics Passed Core Comparison ✅Original model has 15 species. aromatics Failed Edge Comparison ❌Original model has 106 species. Non-identical thermo! ❌
Identical thermo comments: DetailsObservables 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:54 liquid_oxidation Passed Core Comparison ✅Original model has 37 species. liquid_oxidation Failed Edge Comparison ❌Original model has 214 species. Non-identical kinetics! ❌
kinetics: DetailsObservables 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:01 nitrogen Failed 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: DetailsObservables 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:49 oxidation Passed Core Comparison ✅Original model has 59 species. oxidation Passed Edge Comparison ✅Original model has 230 species. DetailsObservables 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 ck2yaml that comes with Cantera 3.1 needs the surface site to be named. Because there's no 3.2 binaries in conda-forge for Python 3.9, we'll end up installing 3.1 in most people's environments, so let's be compatible with it (even if it's wrong).
Because Cantera 3.1 requires this, and at least temporarily we require Cantera v 3.1, we'll add it to our testing and example files.
|
I made it so that chemkin files produced by RMG are compatible with Cantera 3.1, since most people will end up with Cantera 3.1 in their conda environment. I think this is ready. |
Now that we name the surface (for compatibility with Cantera 3.1) we need to be able to read these chemkin files ourself.
Load surface phase name dynamically from generated YAML instead of assuming hardcoded 'surface1'. This makes the code compatible with different CHEMKIN surface file specifications that may define different phase names (e.g., SURF0). Fixes failing tests in canteramodelTest.py.
(It forgot, and wasted a bunch of time trying to fix a bug that wasn't there)
|
Any ideas to get the MacOS builder to not time out? |
This step was fine on the morning |
|
I see it's now running again, and has passed that step: https://github.com/ReactionMechanismGenerator/RMG-Py/actions/runs/21961618247/job/63537644457?pr=2885 |
Regression Testing Results
Detailed regression test results.Regression test aromatics:Reference: Execution time (DD:HH:MM:SS): 00:00:00:56 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: DetailsObservables 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:58 liquid_oxidation Passed Core Comparison ✅Original model has 37 species. liquid_oxidation Failed Edge Comparison ❌Original model has 214 species. DetailsObservables 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 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(Cyclopropene) + radical(CdJ-NdO) Non-identical kinetics! ❌
kinetics: DetailsObservables 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:49 oxidation Passed Core Comparison ✅Original model has 59 species. oxidation Passed Edge Comparison ✅Original model has 230 species. DetailsObservables 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.
|
3 participants
Motivation or Problem
RMG-Py pins Cantera to version 2.6. Cantera 3.0+ removed several deprecated APIs (
ThreeBodyReaction,FalloffReaction,InterfaceReaction,Species.fromCti(),reaction.efficiencies), breaking compatibility.Description of Changes
I took @alongd's commits from PR #2870 (which also adds a Cantera yaml writer).
This PR isolates just the update to Cantera.
=2.6to>=3.0inenvironment.ymlct.ThreeBodyReaction,ct.FalloffReaction, andct.InterfaceReactionwith the unifiedct.Reactionconstructor inrmgpy/reaction.pyThirdBody,Lindemann, andTroeinrmgpy/kinetics/falloff.pyxto access efficiencies viact_reaction.third_body.efficienciesrmgpy/tools/canteramodel.py(ct_rxn.rate.ratesinstead ofct_rxn.rates)rmgpy/rmg/main.pyct.Species.fromCti()toct.Species.from_yaml()TestCanteraOutputclass that relied on removed Cantera 2 APIsos.makedirswithexist_ok=True,permissive=Truefor surface Chemkin loading)Because conda-forge has no Cantera 3.2 for Python 3.9, we end up installing Cantera 3.1.
That version has a regression in the ck2yaml converter that requires the surface sites to be named, so we added this to make RMG-generated chemkin files compatible with it.
Testing
Reviewer Tips
cantera>=3.0in your environment. Probably worth making a whole new conda environment.make installbefore testingTestCanteraOutputclass was removed; Maybe that change should only be on the other PR (that adds the yaml writer). Consider whether equivalent coverage is needed via the new YAML-based workflowOther Context
Jackson started an upgrade in #2751
This would also solve #2676