Reduce CI runtime#2907
Conversation
Our CI takes a long time. This PR will reduce that by taking the following steps: - only test the "with RMS" version of RMG on MacOS platforms, assuming that it still works without it - only test the latest version of Python on MacOS platforms, assuming that older versions will also work - only run the unit tests on macos (no functional or database tests), since they cover most of the codebase anyway - keep ubuntu as is, just to cover our bases if any the above assumptions are wrong These changes are based on the observation that the ubuntu runners are more abundant and faster, so we can rely on them for the heavy lifting. Closes ReactionMechanismGenerator#2906
|
Cool. We could probably also slim the Conda Build matrix for PRs (but build everything for pushes to main)? |
Regression Testing Results
Detailed regression test results.Regression test aromatics:Reference: Execution time (DD:HH:MM:SS): 00:00:00:55 aromatics Passed Core Comparison ✅Original model has 15 species. aromatics Passed Edge Comparison ✅Original model has 106 species. 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:56 liquid_oxidation Failed 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:01 nitrogen Failed Core Comparison ❌Original model has 41 species. nitrogen Failed Edge Comparison ❌Original model has 133 species. 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:43 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.
|
JacksonBurns
force-pushed
the
ci/less_slow
branch
from
4645172 to
63f5b06
Compare
|
@rwest I think this is ready now |
There was a problem hiding this comment.
Pull request overview
This PR updates the GitHub Actions workflows to reduce CI wall-clock time by shrinking the macOS test/build matrix and limiting macOS to lighter test suites while keeping broader coverage on Ubuntu.
Changes:
- Reduce macOS CI combinations by running only the latest Python version and (on macOS) only the “with RMS” configuration.
- Run unit tests only on macOS runners while keeping
make test-allon Ubuntu. - Attempt to reduce Conda build matrix size on pull requests by excluding older Python versions per OS.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
.github/workflows/conda_build.yml |
Adds PR-only matrix exclusions intended to shrink the Conda build matrix. |
.github/workflows/CI.yml |
Shrinks macOS matrix and runs only unit tests on non-Ubuntu runners. |
| event_name: ["${{ contains(github.event_name, 'pull_request') }}"] | ||
| exclude: | ||
| # for PRs just run a single build per platform to save time and resources | ||
| - event_name: true | ||
| os: ubuntu-latest | ||
| python-version: "3.9" |
There was a problem hiding this comment.
matrix.event_name is defined as a quoted expression (e.g., "true"/"false"), but the exclude entries compare against the boolean true. If GitHub Actions treats these as different types, the excludes won’t match and the full PR matrix will still run. Consider making the matrix value a real boolean (remove the quotes) or update the excludes to match the string value (e.g., "true"), and/or rename the axis to something like is_pr to reflect what it stores.
| # only check for optional RMS functionality on ubuntu, just to be safe (and only ubuntu since its the fastest and most abundant) | ||
| - os: macos-15-intel | ||
| include-rms: "" | ||
| - os: macos-latest | ||
| include-rms: "" |
There was a problem hiding this comment.
The comment says RMS functionality is only checked on ubuntu, but the excludes remove the non-RMS configuration on macOS (so macOS runs with RMS only). Please update the comment to match the actual matrix behavior (or adjust the excludes if the comment reflects the intended behavior).
Regression Testing Results
Detailed regression test results.Regression test aromatics:Reference: Execution time (DD:HH:MM:SS): 00:00:00:55 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)CsCsH) + group(Cs-(Cds-Cds)CsCsH) + group(Cs-(Cds-Cds)(Cds-Cds)CsH) + group(Cs-(Cds-Cds)CsHH) + group(Cds-CdsCsCs) + group(Cds-CdsCsH) + group(Cds-CdsCsH) + group(Cds-CdsCsH) + polycyclic(s3_4_5_ene_3) + polycyclic(s2_4_5_diene_1_5) + polycyclic(s3_5_5_ene_1) - ring(Cyclobutene) - ring(Cyclopentane) - ring(Cyclopentene) + radical(cyclopentene-allyl) Non-identical thermo! ❌
thermo: Thermo group additivity estimation: group(Cs-(Cds-Cds)CsCsH) + group(Cs-(Cds-Cds)(Cds-Cds)CsH) + group(Cs-(Cds-Cds)(Cds-Cds)CsH) + group(Cs-CsCsHH) + group(Cds-CdsCsCs) + group(Cds-CdsCsH) + group(Cds-CdsCsH) + group(Cds-CdsCsH) + Estimated bicyclic component: polycyclic(s2_3_5_ane) - ring(Cyclopropane) - ring(Cyclopentane) + ring(Cyclopentene) + ring(Cyclopropane) + polycyclic(s2_3_6_diene_0_3) + Estimated bicyclic component: polycyclic(s3_5_6_ane) - ring(Cyclopentane) - ring(Cyclohexane) + ring(Cyclopentene) + ring(1,4-Cyclohexadiene) - ring(Cyclopropane) - ring(Cyclopentene) - ring(1,4-Cyclohexadiene) + radical(cyclopentene-4) Non-identical thermo! ❌
thermo: Thermo group additivity estimation: group(Cs-CsCsCsH) + group(Cs-(Cds-Cds)CsCsH) + group(Cs-(Cds-Cds)CsCsH) + group(Cs-(Cds-Cds)CsHH) + group(Cds- Cds(Cds-Cds)Cs) + group(Cds-CdsCsH) + group(Cds-CdsCsH) + group(Cds-Cds(Cds-Cds)H) + polycyclic(s2_3_5_ene_1) + polycyclic(s2_3_6_diene_1_3) + Estimated bicyclic component: polycyclic(s3_5_6_ane) - ring(Cyclopentane) - ring(Cyclohexane) + ring(Cyclopentene) + ring(1,3-Cyclohexadiene) - ring(Cyclopropane) - ring(Cyclopentene) - ring(1,3-Cyclohexadiene) + radical(cyclopentene-allyl) Non-identical thermo! ❌
Identical thermo comments: 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:56 liquid_oxidation Failed 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 Passed Core Comparison ✅Original model has 41 species. nitrogen Passed Edge Comparison ✅Original model has 133 species. 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:43 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.
|
Our CI takes a long time. This PR will reduce that by taking the following steps:
These changes are based on the observation that the ubuntu runners are more abundant and faster, so we can rely on them for the heavy lifting.
Let's see how long the CI takes on this PR.
Closes #2906