Replace Yield with StoichSpecies#905
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #905 +/- ##
==========================================
+ Coverage 94.76% 94.91% +0.15%
==========================================
Files 65 64 -1
Lines 3226 3027 -199
==========================================
- Hits 3057 2873 -184
+ Misses 169 154 -15 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
This PR replaces the old Yield type with a new StoichSpecies type across the API, tests, and docs, and updates CI/build tooling (CMake, Docker, GitHub Actions, lcov). It also removes now-unnecessary libc++ flags on Ubuntu/Clang and adds an .lcovrc to smooth coverage runs with GoogleTest.
Changes:
- Introduces
micm::StoichSpeciesas the product-species representation and wires it throughChemicalReaction,ChemicalReactionBuilder, and all tests/tutorials/docs, removing the legacyYieldtype. - Updates build/CI environment: raises the CMake minimum to 3.24, switches GitHub Ubuntu runners to
ubuntu-latest, and changes several Docker and devcontainer base images fromfedora:37tofedora:latest. - Adds
.lcovrcto ignore certain lcov mismatch/inconsistent errors, improving robustness of coverage runs.
Reviewed changes
Copilot reviewed 45 out of 45 changed files in this pull request and generated 10 comments.
Show a summary per file
| File | Description |
|---|---|
| test/unit/solver/test_solver_builder.cpp | Switches reaction products in solver builder unit tests from Yield to StoichSpecies. |
| test/unit/solver/test_rosenbrock_solver_policy.hpp | Updates Rosenbrock solver policy tests to use StoichSpecies for products. |
| test/unit/solver/test_rosenbrock.cpp | Uses StoichSpecies instead of Yield in a Rosenbrock solver tolerance test reaction. |
| test/unit/solver/test_backward_euler.cpp | Replaces Yield with StoichSpecies for backward Euler solver test reactions. |
| test/unit/process/test_process_set_policy.hpp | Migrates process-set policy tests to StoichSpecies, including random-system product vectors. |
| test/unit/process/test_process_configuration.cpp | Updates configuration/scoping tests to include stoich_species.hpp and use StoichSpecies in builder calls. |
| test/unit/process/test_process.cpp | Uses StoichSpecies for chemical reaction products in process construction tests. |
| test/unit/openmp/test_openmp.cpp | Replaces Yield with StoichSpecies in OpenMP unit tests’ toy mechanisms. |
| test/unit/cuda/solver/test_cuda_solver_builder.cpp | Updates CUDA solver builder tests to use micm::StoichSpecies for products. |
| test/tutorial/test_vectorized_matrix_solver.cpp | Tutorial test updated to construct reactions with StoichSpecies products. |
| test/tutorial/test_solver_results.cpp | Uses StoichSpecies in solver results tutorial test reactions. |
| test/tutorial/test_solver_configuration.cpp | Replaces Yield with StoichSpecies in solver configuration tutorial tests. |
| test/tutorial/test_rate_constants_user_defined_by_hand.cpp | Converts the user-defined rate constant tutorial test to StoichSpecies for all products. |
| test/tutorial/test_rate_constants_no_user_defined_by_hand.cpp | Same as above for the non-user-defined variant tutorial. |
| test/tutorial/test_openmp.cpp | Updates tutorial OpenMP example to use micm::StoichSpecies instead of micm::Yield. |
| test/tutorial/test_multiple_grid_cells.cpp | Multiple-grid-cells tutorial now uses micm::StoichSpecies for products. |
| test/tutorial/test_README_example.cpp | README example test switched from Yield to StoichSpecies for consistency with the new API. |
| test/regression/regression_policy.hpp | Regression policy test updated to use micm::StoichSpecies for SOA products. |
| test/regression/RosenbrockChapman/util.hpp | Rosenbrock–Chapman regression utilities now construct processes with micm::StoichSpecies products. |
| test/integration/test_integrated_reaction_rates.cpp | Integration tests for reaction rates now use micm::StoichSpecies for product definitions. |
| test/integration/test_chapman_integration.cpp | Chapman integration tests updated to micm::StoichSpecies in all product lists. |
| test/integration/terminator.hpp | Terminator integration test toy mechanism updated to micm::StoichSpecies. |
| test/integration/analytical_surface_rxn_policy.hpp | Analytical surface reaction policy tests now use micm::StoichSpecies products. |
| test/integration/analytical_policy.hpp | Analytical integration tests (various rate laws and stiff systems) migrated from Yield to StoichSpecies. |
| include/micm/system/yield.hpp | Removes the legacy Yield struct, deprecating it in favor of StoichSpecies. |
| include/micm/system/stoich_species.hpp | Introduces the StoichSpecies struct as the new representation of species plus stoichiometric coefficient. |
| include/micm/process/process.hpp | Switches process headers to include stoich_species.hpp instead of yield.hpp. |
| include/micm/process/phase_transfer_process_builder.hpp | Updates includes to stoich_species.hpp for phase-transfer process builder. |
| include/micm/process/phase_transfer_process.hpp | Likewise, uses stoich_species.hpp in the phase transfer process interface. |
| include/micm/process/chemical_reaction_builder.hpp | Changes SetProducts to accept std::vector<StoichSpecies> and stores StoichSpecies products internally. |
| include/micm/process/chemical_reaction.hpp | Updates ChemicalReaction to store products as std::vector<StoichSpecies>. |
| include/micm/System.hpp | System umbrella header now includes stoich_species.hpp instead of yield.hpp. |
| docs/source/user_guide/user_defined_rate_constant_tutorial.rst | Documentation sample code switched to StoichSpecies and updated reaction list to include new user-defined processes. |
| docker/Dockerfile.publish | Changes base image from fedora:37 to fedora:latest for the publish image. |
| docker/Dockerfile.openmp | Same Fedora base image change for the OpenMP Dockerfile. |
| docker/Dockerfile.mpi | Same Fedora base image change for the MPI Dockerfile. |
| docker/Dockerfile.memcheck | Same Fedora base image change for the memcheck Dockerfile. |
| docker/Dockerfile.docs | Same Fedora base image change for the docs Dockerfile. |
| docker/Dockerfile.coverage | Same Fedora base image change for the coverage Dockerfile. |
| docker/Dockerfile | Same Fedora base image change for the main Dockerfile. |
| README.md | README example code updated to use StoichSpecies in product lists. |
| CMakeLists.txt | Bumps cmake_minimum_required to 3.24 and removes duplicated/obsolete libc++ configuration for Linux/Clang. |
| .lcovrc | New lcov configuration to ignore mismatch/inconsistent errors from GoogleTest macros. |
| .github/workflows/ubuntu.yml | Aligns Ubuntu CI jobs to run on ubuntu-latest for both gcc and clang. |
| .devcontainer/Dockerfile.codespace | Devcontainer/Codespaces base image updated from fedora:37 to fedora:latest. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 46 out of 46 changed files in this pull request and generated 9 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
K20shores
left a comment
There was a problem hiding this comment.
It's all fine with me. An alternative is to just use a std::pair and then we don't have a named type. Not sure if it's better, but throwing it out there
| double coefficient_{ 1.0 }; | ||
| }; | ||
|
|
||
| using Yield [[deprecated("micm::Yield has been renamed to micm::StoichSpecies; please use StoichSpecies instead")]] = StoichSpecies; |
There was a problem hiding this comment.
Oh that's neat. I wasn't aware of deprecataeds existence
There was a problem hiding this comment.
Yeah I also learned the deprecated warning from the copilot. Apparently it goes back to C++14.
About using std::pair, since this is used in SetProducts() (and probably SetReactants() too), I think making it a struct would help make things clearer.
This PR:
YieldwithStoichSpeciesStoichSpeciesoverStoichiometrysince it more clearly represents a pairing of a species with its stoichiometric coefficient, whileStoichiometryfeels more like the general concept. I’m open to renaming it toStoichiometryif the team feels that’s a better fit.libc++compiler flag for Ubuntu.libstdc++is the default for Ubuntu even when using clang--ignore-errors mismatchflag to the lcov commands, to treat the mismatch as a warning rather than a fatal errorStatecopy constructor and copy assignment